jyknight added inline comments.
================
Comment at: lib/Driver/Tools.cpp:2928
@@ +2927,3 @@
+/// Parses the various -fpic/-fPIC/-fpie/-fPIE arguments. Then,
+/// smooshes them together with platform defaults, to decide with
+/// whether this compile should be using PIC mode or not. Returns the
----------------
joerg wrote:
> Drop the last with?
Done.
================
Comment at: lib/Driver/Tools.cpp:2930
@@ +2929,3 @@
+/// whether this compile should be using PIC mode or not. Returns the
+/// answer in a tuple of (RelocationModel, PICLevel, IsPIE).
+static std::tuple<llvm::Reloc::Model, int, bool>
----------------
joerg wrote:
> Not a native speaker, but "returns as tuple" sounds more natural?
>
> Why is the PICLevel signed?
It's signed because I just used 'int' as a default; doesn't seem like it makes
a difference, but made it unsigned.
================
Comment at: lib/Driver/Tools.cpp:2998
@@ +2997,3 @@
+ // both PIC and PIE are disabled. Any PIE option implicitly enables PIC
+ // at the same level.
+ Arg *LastPICArg = Args.getLastArg(options::OPT_fPIC, options::OPT_fno_PIC,
----------------
joerg wrote:
> No need to rant about GCC behavior that much. Just state that older GCC
> versions provided different (inconsistent behavior).
I don't think it's even worth saying that much -- the original comment was from
2012, and isn't really relevant information to anyone reading this. Removed.
================
Comment at: lib/Driver/Tools.cpp:3022
@@ +3021,3 @@
+ // Introduce a Darwin-specific hack. If the default is PIC but the flags
+ // specified while enabling PIC enabled level 1 PIC, just force it back to
+ // level 2 PIC instead. This matches the behavior of Darwin GCC (based on my
----------------
joerg wrote:
> Difficult to parse. Please rewrite the second sentence.
Done.
================
Comment at: lib/Driver/Tools.cpp:3032
@@ +3031,3 @@
+ PIC = PIE = false;
+ if (Args.hasArg(options::OPT_static))
+ PIC = PIE = false;
----------------
joerg wrote:
> This check doesn't make sense to me. When using just -S, -static is
> completely ignored. So why should it be relevant here?
Not sure what you mean? This code isn't only used with -S -- it's used for all
clang compile and assemble actions.
================
Comment at: lib/Driver/Tools.cpp:3087
@@ -2943,1 +3086,3 @@
const ArgList &Args, const char *LinkingOutput) const
{
+ std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
+ const llvm::Triple Triple(TripleStr);
----------------
joerg wrote:
> This part looks like a clean up that can be applied separately first?
Committed separately as r245154.
http://reviews.llvm.org/D11845
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
