-
Notifications
You must be signed in to change notification settings - Fork 212
[aaelf64-morello] Drop pointless GD/IE-to-LE relaxation #356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[aaelf64-morello] Drop pointless GD/IE-to-LE relaxation #356
Conversation
These LE "relaxations" do not change the instruction sequence at all compared with IE, they just change where the 16 bytes are stored, moving them from the GOT to somewhere in a read-only data section. This doesn't achieve anything and just complicates linker implementations to do, when instead they can just do nothing and let the existing implementation recognise that the GOT entries are constants. Thus, remove these relaxations, and note that there is no need for a dynamic relocation in the case where you would normally relax to LE.
smithp35
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I don't have any record that I can find internally as to why it was preferred to use .rodata rather than a .got entry. Only thing that I can think of are trying to avoid a .got when static linking.
I've added @amilendra in case he has any additional thoughts. If not I'll merge by the end of the week.
| ``R_MORELLO_TPREL128`` dynamic relocation. The access must use the | ||
| ``R_MORELLO_TLSIE_ADR_GOTTPREL_PAGE20`` and ``R_MORELLO_TLSIE_ADD_LO12`` | ||
| relocations in order to allow relaxation to Local Exec. There are no other | ||
| relocations. There are no other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could get rid of the full sentence, or possibly change must use to uses
The access must use the
R_MORELLO_TLSIE_ADR_GOTTPREL_PAGE20andR_MORELLO_TLSIE_ADD_LO12
relocations.
Not got a strong opinion as I don't have any other candidates that could be used instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prescriptive is probably better than descriptive for a specification though?
|
|
||
| - changing the ``R_MORELLO_TLSIE_ADD_LO12`` relocation on symbol the ``sym`` to | ||
| a ``R_AARCH64_ADD_ABS_LO12_NC`` relocation with the symbol ``_sym_data``. | ||
| No such relaxation is specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolving the R_MORELLO_TPREL128 at static link time could be considered a relaxation.
Could delete lines 1076 to 1078.
I guess if the IE sequence had been defined with sufficient NOPS (with an identifying null relocation) it may have been possible to replace the IE sequence with the LE MOVK/MOVZ. That's out of scope of this change though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't count it as a linker relaxation, otherwise "not emitting R_AARCH64_ABS64 for PDEs" would also be a relaxation. And yes, the lack of NOPs is the main reason no such relaxation is possible, there's just not enough space to cram in materialising the size for SCBNDS (and you'd likely need more structure on the instruction sequence than currently required).
Probably, but the relocation asks for a GOT entry so you should be prepared to have one. Especially with purecap Morello being a new ABI you don't need to worry about compatibility with existing linker scripts etc. There's some related oddness in the compiler currently too where it'll define its own GOT-like constant pools of capabilities rather than just loading from the GOT. For CHERI it's far simpler to just always use the GOT rather than trying to work around any weird cases where not having a GOT is traditionally expected (and in the case of constant pools of capabilities those are still .data.rel.ro so have the exact same section flags as .got, just a different name for the same thing...).
Great, thanks. I see the Morello LLVM MR has already been merged; technically it violates the spec as written today without this PR, but only barely (since the section name for this magic .rodata-like section is left up to the implementation, .got is a valid name for it; the only way it violates it is the section is writable). |
|
No further comments by end of the week so merging. |
These LE "relaxations" do not change the instruction sequence at all
compared with IE, they just change where the 16 bytes are stored, moving
them from the GOT to somewhere in a read-only data section. This doesn't
achieve anything and just complicates linker implementations to do, when
instead they can just do nothing and let the existing implementation
recognise that the GOT entries are constants.
Thus, remove these relaxations, and note that there is no need for a
dynamic relocation in the case where you would normally relax to LE.