[PATCH] [lld][ARM] Add relocations to perform function calls

Denis Protivensky dprotivensky at accesssoftek.com
Tue Feb 3 01:50:49 PST 2015


================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:35
@@ +34,3 @@
+  return result;
+}
+
----------------
ruiu wrote:
> denis-protivensky wrote:
> > denis-protivensky wrote:
> > > ruiu wrote:
> > > > Is this intentional that you used signed integers instead of unsigned integers in this function? I'd use uint16_t when I do bit manipulation.
> > > Agreed, will update all of these places while merging.
> > I just looked carefully into the code and realized that using signed integers is a proper way of handling addends, because they may be negative numbers as well.
> > As for MOV addend of Thumb instruction you mentioned, it's formed from exactly 16 bits, and the value is a signed integer, which may be negative (as the reference says). So using signed integers is perfectly fine for all relocation types.
> I'd think the final result type, which is a signed integer type, can be different from types used for intermediate computation. We shuffle bits by shifts and bitmasks to form a 16 bit pattern here. All intermediate types except the result can be unsigned, no? Looks like all right bit shifts in this function aren't used to extend sign bit.
Okay, I'll change all value initialization and intermediate result calculations to operate with unsigned ints of the same size.

http://reviews.llvm.org/D7223

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list