[PATCH] D23930: [AArch64] Fix encoding for lsl #12 in add/sub immediates
Peter Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 30 02:49:03 PDT 2016
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
>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.
More information about the llvm-commits