User Details
- User Since
- Jun 25 2018, 2:18 PM (248 w, 1 d)
Yesterday
Yea sure, what email would you like me to use for the commit author email?
I'm not sure if there is special sign off around this file since it's so public facing but this LGTM
Fri, Mar 3
spacing
Move bool into BitcodeCompiler
update function to mutate instead of return
remove redundent return
Thu, Mar 2
Jan 15 2023
Jan 14 2023
Jan 13 2023
Jan 12 2023
Jan 10 2023
Jan 5 2023
Dec 23 2022
Dec 22 2022
Improve variable naming, require originFile existance
Rename CLI arg, add file path to warning / error
Remove literal deduping
Dec 21 2022
Discussion: https://discord.com/channels/636084430946959380/636732801042874388/1055235620844355595
Previous discussion: https://reviews.llvm.org/D117250
Alternatively in this patch we could not include --strict-auto-link-options and we could remove --ignore-auto-link-option
After reading all this discussion, and seeing this well timed issue https://github.com/llvm/llvm-project/issues/59627 which is caused by this as well, I feel like we should take the opportunity to mirror ld64's behavior entirely, and potentially add a flag to opt-in to the more strict behavior that exists today. My opinion changed here since we talked about this last year because at least 5 people who have tried this have asked me about this specific issue. This issue is far more wide-spread than I originally thought in the community, and I don't think there's huge value in fixing these issues if your link succeeds anyways. Overall from this conversation it doesn't seem like anyone feels strongly enough about being strict on this issue that it justifies differing from ld64 and making adoption even just slightly harder.
Dec 16 2022
The benefit of per library opt outs is you can make sure new violations don't sneak in. But either way we can deal with that part separately from this if we want to add a new flag like that
If we make them warnings, you would actually be able to disable the warning today by passing --ignore-auto-link-option=Foo as well, without any new flags
Here's the ld64 mirroring option: https://reviews.llvm.org/D140244
This is one of the options being discussed on https://reviews.llvm.org/D140225
I wonder if at this point we should just implement LD64's behavior with an off-by-default flag to be more strict? Specifically they ignore these failures and collect the missing ones and only print about them in the case the link fails
Dec 15 2022
Dec 13 2022
Improve test naming
Make tests address independent
Dec 12 2022
Fix test addrs
I landed, thanks all!
rebase to run CI
Dec 8 2022
Parameterize tests and add all test cases
Yes it does, and it looks like we get that today with lld because of platform == PLATFORM_MACOS
Dec 7 2022
Dec 5 2022
dropping this one and went for the discussed approach in https://reviews.llvm.org/D139347 which has no overlap with this one
Dec 2 2022
2 thoughts:
It feels to me like we're defeating the purpose of LLVM_HOST_TRIPLE, but I imagine any other solution to this would require a much bigger hammer, so seems worth a try.
I submitted https://reviews.llvm.org/D139215 and https://reviews.llvm.org/D139223 based on this discussion. Please check them out!
this isn't exactly what was discussed in https://reviews.llvm.org/D130221 but it seems like making LinkGraphMaterializationUnit accessible is nicer for unit tests where we only care to test its behavior vs the whole system. lmk what you think. Locally I also have a patch to use MachOPlatform::isInitializerSection instead of the local version here but I wanted to prove the tests pass before and after so I didn't include it yet.
Nov 29 2022
Add x86_64 example showing -no_pie isn't rejected in that case
Nov 28 2022
I'm not super familiar with the history behind LLVM_HOST_TRIPLE, but one thing that definitely worries me about this specific change is there are other consumers of that variable, but only this 1 gets this fixup. I feel like the core issue is the mismatch between having a single LLVM_HOST_TRIPLE, but having cmake build a fat binary.