[PATCH] D114172: [ARM] implement support for ALU/LDR PC-relative group relocations

[Ard Biesheuvel via Phabricator via llvm-commits]

ardb added inline comments.


================
Comment at: lld/ELF/Arch/ARM.cpp:431
+    rem = val;
+    val &= (uint64_t)0xffffffU >> lz;
+  } while (group--);
----------------
MaskRay wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > 
> > val is uint32_t. Remove `(uint64_t)`?
> Is `(uint64_t)0xffffff >> 32` possible? If lz can be 32, casting to `(uint64_t)` LGTM but it needs a comment. I am not familiar with the encoding and do not know whether lz can be 32.
Yes, lz will be 32 if val == 0. This may happen when using multiple groups if the value can be fully encoded by the previous group(s).

Note that in that case, doing &= should not have any effect, but a 32-bit shift of a 32-bit type is better avoided entirely, I'd assume?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114172/new/

https://reviews.llvm.org/D114172