[PATCH] ARM: For thumb fixups store halfwords high first and low second
Saleem Abdulrasool
compnerd at compnerd.org
Thu Apr 17 18:34:19 PDT 2014
Thanks for fixing this.
================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:317
@@ +316,3 @@
+ }
+ else
+ return Value;
----------------
The else is unnecessary.
================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:328
@@ +327,3 @@
+ Value |= FirstHalf;
+ }
+ else {
----------------
The else should be coddled:
} else {
================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:335
@@ +334,3 @@
+ return Value;
+}
+
----------------
This is purely a stylistic nit, and the choice is yours to make, but I find this to be just as readable, but more concise:
static uint32_t JoinHalfWords(uint32_t First, uint32_t Second,
bool IsLittleEndian) {
if (IsLittleEndian)
return (Second & 0xffff << 16) | (First & 0xffff << 0);
return (First & 0xffff << 16) | (Second & 0xffff << 0);
}
================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:326
@@ +325,3 @@
+ if (IsLittleEndian) {
+ Value |= SecondHalf << 16;
+ Value |= FirstHalf;
----------------
Can you please mask the lower halves of the words since you are passing in uint32_t rather than uint16_t?
================
Comment at: test/MC/ARM/thumb2be-b.w-encoding.s:1
@@ +1,2 @@
+// RUN: llvm-mc -triple=thumbebv7-none-linux-gnueabi -mcpu=cortex-a8 -filetype=obj < %s | llvm-objdump -arch=thumbeb -s - | FileCheck %s
+ .syntax unified
----------------
Can you change the test to test both little and big endian? -check-prefix should allow you to check both variants in the same file.
http://reviews.llvm.org/D3380
More information about the llvm-commits
mailing list