[Driver] Report warnings for unclaimed TargetSpecific options for assembler input

[MaskRay]

Summary

This patch amends D151590 to not error for unlaimed TargetSpecific
options for -x assembler input files. This input type causes Driver to
construct tools::ClangAs (-fintegrated-as) or other assemblers (e.g.
tools::gnutools::Assembler) Their ConstructJobs methods, unlike
Clang::ConstructJobs, claim very few options. If an option is unclaimed,
it either leads to a -Wunused-command-line-argument warning or an error
(if TargetSpecific is set):

% clang '-###' --target=aarch64 -mbranch-protection=bti -c a.s
clang: error: unsupported option '-mbranch-protection=' for target 'aarch64'

It seems that downgrading the diagnostic to warning is most useful as
many users use CFLAGS even for .s files:

clang --target=aarch64 -mbranch-protection=bti -S a.c
clang --target=aarch64 -mbranch-protection=bti -c a.s

I decide not to suppress the warning so that
-Wunused-command-line-argument lovers still get a warning, and help
projects use proper ASFLAGS/CFLAGS/etc.

Note: -mbranch-protection=bti a.S currently has no warning as -x assembler-with-cpp
instructs clangDriver to select tools::Clang and claim most options.

Revert D159010 to demonstrate that we emit a warning for -mfpmath= for
-x assembler input.

Modify my AIX cleanup cd18efb61d759405956dbd30e4b5f2720d8e1783 to
add an err_drv_unsupported_opt_for_target.

Planned for main and release/17.x

[thesamesam]

Thank you!

[MaskRay]

Thank you. I am testing more thoroughly. We need this to catch up 17.0.0rc4...

clang/lib/Driver/Driver.cpp

void Driver::BuildJobs(Compilation &C) const {
   (void)C.getArgs().hasArg(options::OPT_fdriver_only);
   // Claim -### here.
   (void)C.getArgs().hasArg(options::OPT__HASH_HASH_HASH);
 
   // Claim --driver-mode, --rsp-quoting, it was handled earlier.
   (void)C.getArgs().hasArg(options::OPT_driver_mode);
   (void)C.getArgs().hasArg(options::OPT_rsp_quoting);
 
+  bool HasAssembleJob = llvm::any_of(C.getJobs(), [](auto &J) {
+    // Match ClangAs and other derived assemblers of Tool. ClangAs uses a
+    // longer ShortName "clang integrated assembler" while other assemblers just
+    // use "assembler".
+    return strstr(J.getCreator().getShortName(), "assembler");
+  });
   for (Arg *A : C.getArgs()) {
     // FIXME: It would be nice to be able to send the argument to the
     // DiagnosticsEngine, so that extra values, position, and so on could be
     // printed.
     if (!A->isClaimed()) {
       if (A->getOption().hasFlag(options::NoArgumentUnused))
         continue;
 
if (!A->isClaimed()) {
         if (DuplicateClaimed)
           continue;
       }
 
       // In clang-cl, don't mention unknown arguments here since they have
       // already been warned about.
       if (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) {
         if (A->getOption().hasFlag(options::TargetSpecific) &&
-            !A->isIgnoredTargetSpecific()) {
+            !A->isIgnoredTargetSpecific() && !HasAssembleJob) {
           Diag(diag::err_drv_unsupported_opt_for_target)
               << A->getSpelling() << getTargetTriple();
         } else {
           Diag(clang::diag::warn_drv_unused_argument)
               << A->getAsString(C.getArgs());
         }
       }
     }

clang/lib/Driver/ToolChains/AIX.cpp

 using namespace llvm::opt;
 using namespace llvm::sys;
 
 void aix::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
                                   const InputInfo &Output,
                                   const InputInfoList &Inputs,
                                   const ArgList &Args,
                                   const char *LinkingOutput) const {
+  const Driver &D = getToolChain().getDriver();
   ArgStringList CmdArgs;
 
   const bool IsArch32Bit = getToolChain().getTriple().isArch32Bit();
   const bool IsArch64Bit = getToolChain().getTriple().isArch64Bit();
   // Only support 32 and 64 bit.
   if (!IsArch32Bit && !IsArch64Bit)
     llvm_unreachable("Unsupported bit width value.");
 
+  if (Arg *A = C.getArgs().getLastArg(options::OPT_G)) {
+    D.Diag(diag::err_drv_unsupported_opt_for_target)
+        << A->getSpelling() << D.getTargetTriple();
+  }
+
   // Specify the mode in which the as(1) command operates.
   if (IsArch32Bit) {
     CmdArgs.push_back("-a32");
   } else {
     // Must be 64-bit, otherwise asserted already.
     CmdArgs.push_back("-a64");
   }
 

clang/lib/Driver/ToolChains/Arch/X86.h

 namespace tools {
 namespace x86 {
 
 std::string getX86TargetCPU(const Driver &D, const llvm::opt::ArgList &Args,
                             const llvm::Triple &Triple);
 
 void getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
                           const llvm::opt::ArgList &Args,
-                          std::vector<llvm::StringRef> &Features, bool ForAS);
+                          std::vector<llvm::StringRef> &Features);
 
 } // end namespace x86
 } // end namespace target
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ARCH_X86_H

clang/lib/Driver/ToolChains/Arch/X86.cpp

std::string x86::getX86TargetCPU(const Driver &D, const ArgList &Args,
   default:
     // Fallback to p4.
     return "pentium4";
   }
 }
 
 void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
                                const ArgList &Args,
-                               std::vector<StringRef> &Features, bool ForAS) {
-  if (ForAS) {
-    // Some target-specific options are only handled in AddX86TargetArgs, which
-    // is not called by ClangAs::ConstructJob. Claim them here.
-    Args.claimAllArgs(options::OPT_mfpmath_EQ);
-  }
-
+                               std::vector<StringRef> &Features) {
   // Claim and report unsupported -mabi=. Note: we don't support "sysv_abi" or
   // "ms_abi" as default function attributes.
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mabi_EQ)) {
     StringRef DefaultAbi = Triple.isOSWindows() ? "ms" : "sysv";
     if (A->getValue() != DefaultAbi)
       D.Diag(diag::err_drv_unsupported_opt_for_target)
           << A->getSpelling() << Triple.getTriple();
   }

clang/lib/Driver/ToolChains/CommonArgs.cpp

case llvm::Triple::systemz:
     break;
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
     aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
     break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
-    x86::getX86TargetFeatures(D, Triple, Args, Features, ForAS);
+    x86::getX86TargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::hexagon:
     hexagon::getHexagonTargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
     getWebAssemblyTargetFeatures(D, Triple, Args, Features);
     break;

clang/test/Driver/aarch64-target-as-march.s

 // RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
 // RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a -Xassembler \
 // RUN: -march=armv8.1-a %s 2>&1 | FileCheck --check-prefix=MULTIPLE-VALUES %s
 
 // MULTIPLE-VALUES: "-target-feature" "+v8.1a
 // MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
 
 /// march to compiler and assembler, we choose the one suited to the input file type
-// RUN: not %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
 // RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
 // RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
 // RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
 
 // TARGET-FEATURE-3: "-target-feature" "+v8.3a"
 // TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4: "-target-feature" "+v8.4a"
 // TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"

clang/test/Driver/target-specific.s

+/// Check that we report a warning instead of an error for target-specific compilation only options.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c %s 2>&1 | FileCheck %s
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard -c -fno-integrated-as %s 2>&1 | FileCheck %s
+
+/// Report a warning if we perform the link phase.
+// RUN: %clang -### --target=aarch64 -faddrsig -mbranch-protection=standard %s 2>&1 | FileCheck %s
+
+// CHECK: warning: argument unused during compilation: '-faddrsig'
+// CHECK: warning: argument unused during compilation: '-mbranch-protection=standard'
+
+/// assembler-with-cpp claims compile only options. Ideally we should emit a warning.
+// RUN: %clang -### -Werror --target=aarch64 -c -faddrsig -mbranch-protection=standard -x assembler-with-cpp %s

clang/test/Driver/x86-mfpmath.c

 // RUN: %clang -### -c --target=x86_64 -mfpmath=sse %s 2>&1 | FileCheck %s
 // CHECK: "-mfpmath" "sse"
 
-/// Don't warn for assembler input.
-// RUN: %clang -### -Werror -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck /dev/null --implicit-check-not='"-mfpmath"'
+// RUN: %clang -### -c --target=x86_64 -mfpmath=sse -x assembler %s 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: argument unused during compilation: '-mfpmath=sse'