thakis created this revision.
thakis added a reviewer: rnk.

This relands r365703 (and r365714), originally reviewed at
https://reviews.llvm.org/D64527.

The problem with the old approach was that clang would now warn about
-Wa flags that the integrated assembler didn't understand even when
-fno-integrated-as was used.

Just checking for that before calling CollectArgsForIntegratedAssembler
still makes clang warn about unused flags with -fno-integrated-as, which
isn't desired either.

Instead, omit just the "unknown flag" diag if the integrated assembler
isn't in use.

One of the new test cases is by nickdesaulniers from
https://reviews.llvm.org/D64655.


https://reviews.llvm.org/D65108

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/as-options.s

Index: clang/test/Driver/as-options.s
===================================================================
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -35,3 +35,45 @@
 // RUN:   | FileCheck %s
 
 // CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mincremental-linker-compatible -E \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabi -E \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+// RUN:   -o /dev/null -x c++ %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E \
+// RUN:   -o /dev/null -x assembler-with-cpp %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// NOWARN-NOT: unused
+
+// Test that unsupported arguments do not cause errors when -fno-integrated-as
+// is set.
+// RUN: %clang -Wa,-mno-warn-deprecated -fno-integrated-as %s -S 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOERROR --allow-empty %s
+// NOERROR-NOT: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,'
+
+// -Wa flags shouldn't cause warnings without an assembler stage with
+// -fno-integrated-as either.
+// RUN: %clang -Wa,-mno-warn-deprecated -fno-integrated-as %s -S 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+
+// But -m flags for the integrated assembler _should_ warn if the integrated
+// assembler is not in use.
+// RUN: %clang -mrelax-all -fintegrated-as %s -S 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
+// RUN: %clang -mrelax-all -fno-integrated-as %s -S 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN: unused
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2045,38 +2045,42 @@
 static void CollectArgsForIntegratedAssembler(Compilation &C,
                                               const ArgList &Args,
                                               ArgStringList &CmdArgs,
-                                              const Driver &D) {
-  if (UseRelaxAll(C, Args))
-    CmdArgs.push_back("-mrelax-all");
-
-  // Only default to -mincremental-linker-compatible if we think we are
-  // targeting the MSVC linker.
-  bool DefaultIncrementalLinkerCompatible =
-      C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
-  if (Args.hasFlag(options::OPT_mincremental_linker_compatible,
-                   options::OPT_mno_incremental_linker_compatible,
-                   DefaultIncrementalLinkerCompatible))
-    CmdArgs.push_back("-mincremental-linker-compatible");
-
-  switch (C.getDefaultToolChain().getArch()) {
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-    if (Arg *A = Args.getLastArg(options::OPT_mimplicit_it_EQ)) {
-      StringRef Value = A->getValue();
-      if (Value == "always" || Value == "never" || Value == "arm" ||
-          Value == "thumb") {
-        CmdArgs.push_back("-mllvm");
-        CmdArgs.push_back(Args.MakeArgString("-arm-implicit-it=" + Value));
-      } else {
-        D.Diag(diag::err_drv_unsupported_option_argument)
-            << A->getOption().getName() << Value;
+                                              const Driver &D,
+                                              bool IsIntegratedAs) {
+  // Claim flags for the integrated assembler only if it's being used.
+  if (IsIntegratedAs) {
+    if (UseRelaxAll(C, Args))
+      CmdArgs.push_back("-mrelax-all");
+
+    // Only default to -mincremental-linker-compatible if we think we are
+    // targeting the MSVC linker.
+    bool DefaultIncrementalLinkerCompatible =
+        C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+    if (Args.hasFlag(options::OPT_mincremental_linker_compatible,
+                     options::OPT_mno_incremental_linker_compatible,
+                     DefaultIncrementalLinkerCompatible))
+      CmdArgs.push_back("-mincremental-linker-compatible");
+
+    switch (C.getDefaultToolChain().getArch()) {
+    case llvm::Triple::arm:
+    case llvm::Triple::armeb:
+    case llvm::Triple::thumb:
+    case llvm::Triple::thumbeb:
+      if (Arg *A = Args.getLastArg(options::OPT_mimplicit_it_EQ)) {
+        StringRef Value = A->getValue();
+        if (Value == "always" || Value == "never" || Value == "arm" ||
+            Value == "thumb") {
+          CmdArgs.push_back("-mllvm");
+          CmdArgs.push_back(Args.MakeArgString("-arm-implicit-it=" + Value));
+        } else {
+          D.Diag(diag::err_drv_unsupported_option_argument)
+              << A->getOption().getName() << Value;
+        }
       }
+      break;
+    default:
+      break;
     }
-    break;
-  default:
-    break;
   }
 
   // When passing -I arguments to the assembler we sometimes need to
@@ -2100,6 +2104,11 @@
         continue;
       }
 
+      // External assemblers have arbitrary flags, don't try to parse them --
+      // claiming the -Wa flags is sufficient.
+      if (!IsIntegratedAs)
+        continue;
+
       if (C.getDefaultToolChain().getTriple().isOSBinFormatCOFF() &&
           Value == "-mbig-obj")
         continue; // LLVM handles bigobj automatically
@@ -3543,6 +3552,19 @@
   // Select the appropriate action.
   RewriteKind rewriteKind = RK_None;
 
+  // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+  // with a dummy args list to mark assembler flags as used even when not
+  // running an assembler. Otherwise, clang would emit "argument unused"
+  // warnings for assembler flags when e.g. adding "-E" to flags while debugging
+  // something. That'd be somewhat inconvenient, and it's also inconsistent with
+  // most other flags -- we don't warn on -ffunction-sections not being used
+  // in -E mode either for example, even though it's not really used either.
+  if (!isa<AssembleJobAction>(JA)) {
+    ArgStringList DummyArgs;
+    CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D,
+                                      TC.useIntegratedAs());
+  }
+
   if (isa<AnalyzeJobAction>(JA)) {
     assert(JA.getType() == types::TY_Plist && "Invalid output type.");
     CmdArgs.push_back("-analyze");
@@ -3560,7 +3582,9 @@
   } else if (isa<AssembleJobAction>(JA)) {
     CmdArgs.push_back("-emit-obj");
 
-    CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D);
+    assert(TC.useIntegratedAs());
+    CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D,
+                                      /*IsIntegratedAs=*/true);
 
     // Also ignore explicit -force_cpusubtype_ALL option.
     (void)Args.hasArg(options::OPT_force__cpusubtype__ALL);
@@ -6194,8 +6218,8 @@
   // FIXME: Stop lying and consume only the appropriate driver flags
   Args.ClaimAllArgs(options::OPT_W_Group);
 
-  CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
-                                    getToolChain().getDriver());
+  CollectArgsForIntegratedAssembler(
+      C, Args, CmdArgs, getToolChain().getDriver(), /*IsIntegratedAs=*/true);
 
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to