[PATCH] Implementation of R_ARM_TARGET1

Leny Kholodov leny.kholodov at gmail.com
Tue Mar 31 06:05:55 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Driver/GnuLdDriver.cpp:430
@@ +429,3 @@
+    if (arg->getOption().matches(OPT_grp_targetopts))
+      ctx->applyTargetSpecificArgument(*arg, diag);
+
----------------
ruiu wrote:
> There's no ambiguity on command line option meaning (--arm-target1-rel and --arm-target1-abs clearly for ARM only). Also in this context we already know what the target architecture is. So I don't think we need to abstract the thing away from the driver.
> 
> You can just check value of "triple" to see if it's ARM, and if it's ARM, call setTargetRel1(true or false) directly here. If it's not ARM, print out a warning message.
Ok. I'll move all command line processing stuff to driver class. Also, I'll remove applyTargetSpecificArg polymorphism.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMLinkingContext.cpp:40
@@ +39,3 @@
+  if (arg.getOption().getName() == "arm-target1-rel")
+  {
+    setTarget1Rel(true);
----------------
ruiu wrote:
> This parenthesis position style doesn't follow the LLVM coding style.
Will be fixed.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMLinkingContext.h:56
@@ -47,1 +55,3 @@
+private:
+  bool _target1Rel;
 };
----------------
ruiu wrote:
> Let's use C++11 initializer.
> 
>   bool _target1Rel = false;
WIll be fixed.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h:26
@@ +25,3 @@
+                             ARMLinkingContext &context)
+      : _armLayout(layout),
+        _armContext(context) {}
----------------
ruiu wrote:
> Remove newline after comma.
> 
> Maybe you want to pass a boolean value instead of a context object itself? That makes it clear that you only need the boolean flag.
You are right. Will be fixed.

http://reviews.llvm.org/D8707

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






More information about the llvm-commits mailing list