[PATCH] D51030: [AArch64] Optimise load(adr address) to ldr address

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In https://reviews.llvm.org/D51030#1209332, @peter.smith wrote:

> I think that this is likely to be the only time in the test suite that there is a R_AARCH64_LD_PREL_LO19 to a data symbol in a shared library. In theory this should generate a R_AARCH64_COPY dynamic relocation. I think that this was fixed as part of the changes made in https://sourceware.org/bugzilla/show_bug.cgi?id=21532 "[AArch64] Allow COPY relocation elimination" as prior to that R_AARCH64_LD_PREL_LO19 wasn't in the switch statement to generate copy relocations. The change made it into binutils 2.29.


Thanks for looking. What is our usual response with issues like this? Is it OK for this to go ahead, as it can be used fine in some linkers. I could put it behind a flag I think, so that users have the option to turn it off (although that might not be very obvious..)

> I'm guessing that this is the relaxation from TLSIE to TLSLE? I think that will be possible as it should be possible to tell from the relocation at the place which code model is being used.

Yep :)



================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1098
 
+bool AArch64DAGToDAGISel::tryAdrLoad(SDNode *N) {
+  // Look for a load(adr <addr>) where the addr is 4 byte aligned
----------------
john.brawn wrote:
> I'm pretty sure we can do the selection entirely in tablegen. I experimented with adjusting LoadLiteral AArch64InstrFormats.td to be kinda similar to Load32RO (take ValueType and SDPatternOperator as arguments, use these in a pattern but match AArch64adr for the address) and that seemed to work. The only problem there is making sure the alignment is correct, which I think can be done using a PatFrag or PatLeaf (though I tried something quick and ran into tablegen errors, so maybe it's more complicated than I expect).
Yep, my original version of this was just in tablegen. I wasn't sure about how to do the alignment though, and it seemed to need an addedcomplexity of at least 20, at least the way I did it.

I'll try and take another go at doing it there, so long as it can handle the alignment and all the LDRSWl/LDRXl/LDRWl's. I'll let you know how it looks.


https://reviews.llvm.org/D51030