[PATCH] Implementation of R_ARM_TARGET1
Rui Ueyama
ruiu at google.com
Mon Mar 30 14:39:39 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);
+
----------------
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.
================
Comment at: lib/ReaderWriter/ELF/ARM/ARMLinkingContext.cpp:40
@@ +39,3 @@
+ if (arg.getOption().getName() == "arm-target1-rel")
+ {
+ setTarget1Rel(true);
----------------
This parenthesis position style doesn't follow the LLVM coding style.
================
Comment at: lib/ReaderWriter/ELF/ARM/ARMLinkingContext.h:56
@@ -47,1 +55,3 @@
+private:
+ bool _target1Rel;
};
----------------
Let's use C++11 initializer.
bool _target1Rel = false;
================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h:26
@@ +25,3 @@
+ ARMLinkingContext &context)
+ : _armLayout(layout),
+ _armContext(context) {}
----------------
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.
http://reviews.llvm.org/D8707
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list