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

Denis Protivensky dprotivensky at accesssoftek.com
Mon Feb 2 22:42:18 PST 2015


================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:19
@@ -18,1 +18,3 @@
 
+#define DECLARE_T(flag) const uint64_t T = flag ? 1 : 0;
+
----------------
ruiu wrote:
> Do we need this macro? It would be more readable if we just write
> 
>   uint64_t T = flag;
> 
> instead of
> 
>   DECLARE_T(flag);
Indeed, the implicit conversion from bool should give proper values.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:35
@@ +34,3 @@
+  return result;
+}
+
----------------
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.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:170
@@ +169,3 @@
+  DEBUG_WITH_TYPE(
+      "ARM", llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
+      llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
----------------
ruiu wrote:
> It's just a suggestion and shouldn't be mixed with this patch, but if you define DEBUG_TYPE like this
> 
>   #define DEBUG_TYPE "ARM"
> 
> then you can just use DEBUG(...) instead of DEBUG_WITH_TYPE("ARM", ...).
Thanks, got you.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:295
@@ +294,3 @@
+static void relocR_ARM_THM_MOVW_ABS_NC(uint8_t *location, uint64_t P,
+                                   uint64_t S, int64_t A, bool addressesThumb) {
+  DECLARE_T(addressesThumb);
----------------
ruiu wrote:
> Indentation.
Will fix.

http://reviews.llvm.org/D7223

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






More information about the llvm-commits mailing list