I tested the binary a bit, llvm-bolt (with instrumentation) is working, but perf2bolt seems to be a bit problematic and I am not sure why. There is no error, but the profile result is not useful.

I would create an upstream PR for that part of the patch and mention it here https://discourse.llvm.org/t/building-llvm-out-of-per-subproject-tar-balls-still-is-not-working/72829/11

I have helped shepherd such things upstream before.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3555

We probably need to add this for LLVM 16 as well until Darwin's default LLVM version has caught up, for the same reasons as in ee9c2b7.

I think the patch should work for LLVM 16, but may need some minor tweaks. I can try to add it later today.

It'd also be fine to have it be marked broken for LLVM 16. The attribute just needs to exist for silly reasons. mlir is a good example of how to have a common package definition between multiple LLVM versions, btw.

Added the attribute for llvm 16 as well. It does not work because there is a large cmake file change in llvm 17, need large effort to make it work for llvm 16...

btw not sure how should I name the commit(s), I can do a rebase later if needed.
while this adds bolt for llvm 16 as well, it is marked as broken so not sure if this counts as adding the package

I think the commit message should be titled "llvmPackages.bolt: init at 17.0.6", since there's no "llvm-bolt" attribute at the top level. In the commit message body you could explain that there needs to be an llvmPackages_16.bolt attribute, even if it doesn't work, referencing the commit I referred to before.

BTW: what about the upstream PR @Ericson2314 suggested?

Curious as to what the status of this is since I know someone who is wanting this in LLVM 18. If this is merged before 18 is merged then I can merge this into 18 and git.

This is working, but perhaps people are hesitant to merge it due to the relatively large patch?

Maybe, I'd consider this to be kinda small. I don't see anything bad and I'm testing this on an M1 Pro running NixOS. Only thing I can think of is adding bolt_17 and bolt_16 as aliases to the bolt packages in llvmPackages_16/17. I also didn't see it run any sort of tests, maybe getting it to run tests would be good.

I thought of running tests, but it seems that bolt tests are disabled when not built together with clang. I can look into it later when I have more time.

Alright then just add the aliases and I'd say this is all good.

Do note that bolt_16 is broken, so I think we should only add bolt_17?

I mean do you have a link for here? Or else do I find the upstream PR?

@pca006132 Yes, you can find it by scrolling up and seeing the line that says this PR was mentioned in another one.

@alyssais I will wait until the upstream give more comments about the patch and fix it all together.

Discussion in that upstream PR has reminded me that we should not build the runtime as part of bolt, but in a separate package instead (just like LLVM and compiler-rt), in order to be cross friendly.

Ping, this has an approval, is this still blocked, if so, on what? Can it be merged as-is and then improved?

@pwaller Probably missing link from the upstream PR. I wanted to add a reproducer for the upstream PR using nix (because I somehow cannot make the docker build works), but I am stuck with the lit script. Was busy last month, and I am not at home right now, will revisit this next week.

@pca006132 Is there any interest or progress in updating this PR?

Sorry but probably no, I am busy with other projects.

I can close this one and let you open a new one?

I can close this one and let you open a new one?

Yeah, that sounds good. I already rebased this in my own fork to work with the most recent changes.

Thanks a lot! And really sorry about having no progress for months.

Yeah, it's all good. I've opened #326240.