Fix for PR33096.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
LLD fix coming in a separate patch.
I've opted not to change current behavior for shared and executable linking, but now I'm having second thoughts. Non-LTO pipeline allows the user to put PIC code in a non-PIE executable, or non-PIC code in a shared library if they wish so. Maybe we should completely ignore the output type here and strictly go by module flags, to be consistent.
Non-LTO pipeline allows the user to put PIC code in a non-PIE executable, or non-PIC code in a shared library if they wish so. Maybe we should completely ignore the output type here and strictly go by module flags, to be consistent.
Disabling PIC can sometimes act as an optimization. We may want to replace the reloc model field in lto::Config with a boolean that gives the backend permission to generate position-dependent code. Before we do that though, I think we will need to decide what to do about the other reloc models (dynamic-no-pic, rwpi, ropi, ropi-rwpi).
lib/LTO/LTO.cpp | ||
---|---|---|
117–120 | I'd rewrite this with if/else and drop the .hasValue() here. | |
lib/LTO/LTOBackend.cpp | ||
139 | Can you pass the module into createTargetMachine and inline this function into it? | |
140 | Drop .hasValue(). |
Disabling PIC was a performance hack on 32bit x86 due to the extreme register starvation of the architecture.
It is also generally agreed that it is fundamentally broken from a design perspective. I don't see good reasons for supporting that, frankly.
Disabling PIC on its own doesn't seem that problematic to me. The problematic part is the assumption in the backend that symbols are DSO-local, which leads to hacks such as copy relocations. But with LTO the linker knows whether each symbol is DSO-local, and it could in principle pass that information to the backend, which could then generate appropriate code for accessing each symbol. This is blocked on something like D20217 though.
LGTM with one nit
lib/LTO/LTOBackend.cpp | ||
---|---|---|
119–120 | Remove TheTriple argument, it can always be computed from the module. |
I'd rewrite this with if/else and drop the .hasValue() here.