r187156 - Don't forward all assembler arguments untouched to -cc1as

David Blaikie dblaikie at gmail.com
Thu Jul 25 14:19:01 PDT 2013


Author: dblaikie
Date: Thu Jul 25 16:19:01 2013
New Revision: 187156

URL: http://llvm.org/viewvc/llvm-project?rev=187156&view=rev
Log:
Don't forward all assembler arguments untouched to -cc1as

Use the same filtering for assembly arguments to -cc1as as we do for
-cc1, this allows a consistent (& more useful) diagnostic experience for
users (rather than getting an error from -cc1as (which a user shouldn't
really be thinking about) about --foo, they get an error from clang
about --foo in -Wa,)

I'm sort of surprised by the separation of -cc1as & the separate
argument handling, etc, but at least this removes a little bit of the
duplication.

Modified:
    cfe/trunk/include/clang/Driver/CC1AsOptions.td
    cfe/trunk/lib/Driver/Tools.cpp
    cfe/trunk/test/Driver/integrated-as.c
    cfe/trunk/test/Driver/integrated-as.s
    cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/include/clang/Driver/CC1AsOptions.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1AsOptions.td?rev=187156&r1=187155&r2=187156&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/CC1AsOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CC1AsOptions.td Thu Jul 25 16:19:01 2013
@@ -77,15 +77,12 @@ def show_inst : Flag<["-"], "show-inst">
 // Assemble Options
 //===----------------------------------------------------------------------===//
 
-def relax_all : Flag<["-"], "relax-all">,
+def mrelax_all : Flag<["-"], "mrelax-all">,
     HelpText<"Relax all fixups (for performance testing)">;
 
-def no_exec_stack : Flag<["--"], "noexecstack">,
+def mno_exec_stack : Flag<["-"], "mnoexecstack">,
     HelpText<"Mark the file as not needing an executable stack">;
 
-def fatal_warnings : Flag<["--"], "fatal-warnings">,
-    HelpText<"Consider warnings as errors">;
-
 def g : Flag<["-"], "g">, HelpText<"Generate source level debug information">;
 
 def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=187156&r1=187155&r2=187156&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Jul 25 16:19:01 2013
@@ -1570,6 +1570,41 @@ static bool UseRelaxAll(Compilation &C,
     RelaxDefault);
 }
 
+static void CollectArgsForIntegratedAssembler(Compilation &C,
+                                              const ArgList &Args,
+                                              ArgStringList &CmdArgs,
+                                              const Driver &D) {
+    if (UseRelaxAll(C, Args))
+      CmdArgs.push_back("-mrelax-all");
+
+    // When using an integrated assembler, translate -Wa, and -Xassembler
+    // options.
+    for (arg_iterator it = Args.filtered_begin(options::OPT_Wa_COMMA,
+                                               options::OPT_Xassembler),
+           ie = Args.filtered_end(); it != ie; ++it) {
+      const Arg *A = *it;
+      A->claim();
+
+      for (unsigned i = 0, e = A->getNumValues(); i != e; ++i) {
+        StringRef Value = A->getValue(i);
+
+        if (Value == "-force_cpusubtype_ALL") {
+          // Do nothing, this is the default and we don't support anything else.
+        } else if (Value == "-L") {
+          CmdArgs.push_back("-msave-temp-labels");
+        } else if (Value == "--fatal-warnings") {
+          CmdArgs.push_back("-mllvm");
+          CmdArgs.push_back("-fatal-assembler-warnings");
+        } else if (Value == "--noexecstack") {
+          CmdArgs.push_back("-mnoexecstack");
+        } else {
+          D.Diag(diag::err_drv_unsupported_option_argument)
+            << A->getOption().getName() << Value;
+        }
+      }
+    }
+}
+
 SanitizerArgs::SanitizerArgs(const ToolChain &TC, const ArgList &Args)
     : Kind(0), BlacklistFile(""), MsanTrackOrigins(false),
       AsanZeroBaseShadow(false) {
@@ -1970,35 +2005,7 @@ void Clang::ConstructJob(Compilation &C,
   } else if (isa<AssembleJobAction>(JA)) {
     CmdArgs.push_back("-emit-obj");
 
-    if (UseRelaxAll(C, Args))
-      CmdArgs.push_back("-mrelax-all");
-
-    // When using an integrated assembler, translate -Wa, and -Xassembler
-    // options.
-    for (arg_iterator it = Args.filtered_begin(options::OPT_Wa_COMMA,
-                                               options::OPT_Xassembler),
-           ie = Args.filtered_end(); it != ie; ++it) {
-      const Arg *A = *it;
-      A->claim();
-
-      for (unsigned i = 0, e = A->getNumValues(); i != e; ++i) {
-        StringRef Value = A->getValue(i);
-
-        if (Value == "-force_cpusubtype_ALL") {
-          // Do nothing, this is the default and we don't support anything else.
-        } else if (Value == "-L") {
-          CmdArgs.push_back("-msave-temp-labels");
-        } else if (Value == "--fatal-warnings") {
-          CmdArgs.push_back("-mllvm");
-          CmdArgs.push_back("-fatal-assembler-warnings");
-        } else if (Value == "--noexecstack") {
-          CmdArgs.push_back("-mnoexecstack");
-        } else {
-          D.Diag(diag::err_drv_unsupported_option_argument)
-            << A->getOption().getName() << Value;
-        }
-      }
-    }
+    CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D);
 
     // Also ignore explicit -force_cpusubtype_ALL option.
     (void) Args.hasArg(options::OPT_force__cpusubtype__ALL);
@@ -3816,9 +3823,6 @@ void ClangAs::ConstructJob(Compilation &
   CmdArgs.push_back("-main-file-name");
   CmdArgs.push_back(Clang::getBaseInputName(Args, Inputs));
 
-  if (UseRelaxAll(C, Args))
-    CmdArgs.push_back("-relax-all");
-
   // Add target specific cpu and features flags.
   switch(getToolChain().getArch()) {
   default:
@@ -3884,8 +3888,9 @@ void ClangAs::ConstructJob(Compilation &
 
   // FIXME: Add -static support, once we have it.
 
-  Args.AddAllArgValues(CmdArgs, options::OPT_Wa_COMMA,
-                       options::OPT_Xassembler);
+  CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
+                                    getToolChain().getDriver());
+
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
   assert(Output.isFilename() && "Unexpected lipo output.");

Modified: cfe/trunk/test/Driver/integrated-as.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/integrated-as.c?rev=187156&r1=187155&r2=187156&view=diff
==============================================================================
--- cfe/trunk/test/Driver/integrated-as.c (original)
+++ cfe/trunk/test/Driver/integrated-as.c Thu Jul 25 16:19:01 2013
@@ -4,4 +4,4 @@
 // REQUIRES: clang-driver
 
 // CHECK: cc1as
-// CHECK: -relax-all
+// CHECK: -mrelax-all

Modified: cfe/trunk/test/Driver/integrated-as.s
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/integrated-as.s?rev=187156&r1=187155&r2=187156&view=diff
==============================================================================
--- cfe/trunk/test/Driver/integrated-as.s (original)
+++ cfe/trunk/test/Driver/integrated-as.s Thu Jul 25 16:19:01 2013
@@ -4,3 +4,6 @@
 
 // CHECK: cc1as
 // CHECK-NOT: -relax-all
+
+// RUN: %clang -c -integrated-as -Wa,--compress-debug-sections %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID: error: unsupported argument '--compress-debug-sections' to option 'Wa,'

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=187156&r1=187155&r2=187156&view=diff
==============================================================================
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Thu Jul 25 16:19:01 2013
@@ -206,8 +206,6 @@ bool AssemblerInvocation::CreateFromArgs
     }
   }
   Opts.LLVMArgs = Args->getAllArgValues(OPT_mllvm);
-  if (Args->hasArg(OPT_fatal_warnings))
-    Opts.LLVMArgs.push_back("-fatal-assembler-warnings");
   Opts.OutputPath = Args->getLastArgValue(OPT_o);
   if (Arg *A = Args->getLastArg(OPT_filetype)) {
     StringRef Name = A->getValue();
@@ -233,8 +231,8 @@ bool AssemblerInvocation::CreateFromArgs
   Opts.ShowInst = Args->hasArg(OPT_show_inst);
 
   // Assemble Options
-  Opts.RelaxAll = Args->hasArg(OPT_relax_all);
-  Opts.NoExecStack =  Args->hasArg(OPT_no_exec_stack);
+  Opts.RelaxAll = Args->hasArg(OPT_mrelax_all);
+  Opts.NoExecStack =  Args->hasArg(OPT_mno_exec_stack);
 
   return Success;
 }





More information about the cfe-commits mailing list