[PATCH] D23930: [AArch64] Fix encoding for lsl #12 in add/sub immediates

[Peter Smith via llvm-commits]

peter.smith added a subscriber: peter.smith.
peter.smith added a comment.

>From an ELF perspective I think that this is the right thing to do. The only OpenSource ELF linker I know of that implements  R_AARCH64_TLSLE_ADD_TPREL_HI12 does not touch the shift field when resolving the relocation. It is possible to write a test case that gives an incorrect result when there is a large enough amount of tls for the tprel_hi12 to be needed.

One thing I note that the GNU assembler will do is set the shift value if the result of the expression can be represented with the shift, but not without it. For example
.https://reviews.llvm.org/L1:
 .space 0x10000
.L2:
>From my reading of the code it seems like we'll fault this as out of range. If I'm right it would be nice to support this, but I don't think it necessarily needs to be done as part of this one.


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp:266
@@ -265,3 +265,3 @@
 
-  return 0;
+  return ShiftVal == 0 ? 0 : (1 << ShiftVal);
 }
----------------
Line 256 has an almost identical (ShiftVal == 0 ? 0 : (1 << 12)), it could be worth being consistent with this line although I don't have a particularly strong opinion.


https://reviews.llvm.org/D23930