[PATCH] ARM: For thumb fixups store halfwords high first and low second

Saleem Abdulrasool compnerd at compnerd.org
Wed Apr 16 10:17:12 PDT 2014



================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:309
@@ -308,1 +308,3 @@
 
+static uint64_t swapFixupValue(uint64_t Value, bool IsLittleEndian) {
+  if (IsLittleEndian) {
----------------
Why not call it SwapHalfWords?  It isnt tied to a MCFixup.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:358
@@ -344,5 +357,3 @@
     Value = (Hi4 << 16) | (i << 26) | (Mid3 << 12) | (Lo8);
-    uint64_t swapped = (Value & 0xFFFF0000) >> 16;
-    swapped |= (Value & 0x0000FFFF) << 16;
-    return swapped;
+    return swapFixupValue (Value, IsLittleEndian);
   }
----------------
No space after the function name.  Likewise elsewhere.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:484
@@ -481,3 +483,3 @@
                           (uint16_t)imm11Bits);
-    Binary |= secondHalf << 16;
-    Binary |= firstHalf;
+    if (IsLittleEndian) {
+      Binary |= secondHalf << 16;
----------------
I think an auxiliary function like:

    uint32_t JoinHalfWords(uint16_t First, uint16_t Second, bool IsLittleEndian);

might be nice in this case since we could reuse it in the cases where we need.

Also, the else should be coddled:

    } else {


http://reviews.llvm.org/D3380






More information about the llvm-commits mailing list