[PATCH] D23053: [COFF][ARM] Clear the J1 and J2 bits when applying relocations to 24 bit branches

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 4 22:26:06 PDT 2016


mstorsjo added inline comments.

================
Comment at: COFF/Chunks.cpp:107
@@ +106,3 @@
+  // The J1 and J2 bits are set in the opcode already, so mask them out
+  // before or'ing in the new bits.
+  write16le(Off + 2, (read16le(Off + 2) & 0xd000) | (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
----------------
compnerd wrote:
> Yeah, the bits need to be cleared out as the value may be altered by the new computation.
> 
> There is no guarantee that the bits are set IIRC, so I find the comment misleading.  You can add a comment along the lines of `clear out the J1 and J2 bits which may be set`, though, Im not sure if it really adds much value.
Ok, uploading a new version of the patch with the comment changed.


https://reviews.llvm.org/D23053





More information about the llvm-commits mailing list