[Driver] Add ClangFlags::TargetSpecific to simplify err_drv_unsupported_opt_for_target processing

[MaskRay]

Summary

clang/lib/Driver/ToolChains/Clang.cpp has a lot of fragments like the following:

if (const Arg *A = Args.getLastArg(...)) {
  if (Triple is xxx)
    A->render(Args, CmdArgs);
  else
    D.Diag(diag::err_drv_unsupported_opt_for_target) << ...;
}

The problem is more apparent with a recent surge of AIX-specific options.

Introduce the TargetSpecific flag so that we can move the target-specific
options to ToolChains/*.cpp and ToolChains/Arch/*.cpp and overload the
warn_drv_unused_argument mechanism to give an err_drv_unsupported_opt_for_target
error.

Migrate -march=/-mcpu= and some AIX-specific options to use this simplified pattern.

[qiongsiwu1]

Thanks for the patch!

Recently we added the options mxcoff-roptr and mno-xcoff-roptr https://reviews.llvm.org/D144190 that fall into this the scope of this patch (e.g. https://github.com/llvm/llvm-project/blob/0508ac32cfcc1a9fc5a81d16de8d418dc5a0666b/clang/lib/Driver/ToolChains/Clang.cpp#LL5278C19-L5278C19), since they are only supported on AIX. Could you add the handling of options mxcoff-roptr and mno-xcoff-roptr?

[MaskRay]

In D151590#4379087, @qiongsiwu1 wrote:

Thanks for the patch!

Recently we added the options mxcoff-roptr and mno-xcoff-roptr https://reviews.llvm.org/D144190 that fall into this the scope of this patch (e.g. https://github.com/llvm/llvm-project/blob/0508ac32cfcc1a9fc5a81d16de8d418dc5a0666b/clang/lib/Driver/ToolChains/Clang.cpp#LL5278C19-L5278C19), since they are only supported on AIX. Could you add the handling of options mxcoff-roptr and mno-xcoff-roptr?

Hi @qiongsiwu1, thanks for chiming in. I am aware of -mxcoff-roptr. This patch picks some example patterns, but does not attempt to be comprehensive (on the GNU side, these are many many -m options that are not covered. E.g. LoongArch has a -mabi hack, so I cannot make -mabi TargetSpecific yet)
I can fold the -mxcoff-roptr change into this patch since it's straightforward.

[MaskRay]

Handle -mxcoff-roptr as well

[jansvoboda11]

Nice improvement! Some tests seem to be failing due to the changes, but once those are fixed, LGTM.

[MaskRay]

Rebase.
Fix amdgpu. I suspect D145579 introduced latent -march= and -mcpu= interaction issues but I will leave the test coverage to amdgpu folks.

[MaskRay]

@jansvoboda11

For ELF operating systems (that traditionally use GCC), I think unsupported -m* options as errors are very desired as that's the GCC behavior and some configure system will prefer it. (For instance, they need to pass -Werror -mxxx to test availability, instead of just -mxxx.)

I think x86 -m* options are the most possible options to be misused.

% clang -c --target=aarch64-unknown-linux-gnu -msse4.2 a.c    # warning instead of error, not ideal
clang: warning: argument unused during compilation: '-msse4.2' [-Wunused-command-line-argument]
% aarch64-linux-gnu-gcc -msse4.2 a.c
aarch64-linux-gnu-gcc: error: unrecognized command-line option ‘-msse4.2’

For macOS universal binary builds (-arch), I wonder whether we want to refine the current behavior:

% clang -fdriver-only -c --target=x86_64-apple-darwin -arch arm64 -arch x86_64 -mavx512vl a.c   # no warning
% clang -fdriver-only -c --target=x86_64-apple-darwin -arch arm64 -mavx512vl a.c   # no -arch x86_64
clang: warning: argument unused during compilation: '-mavx512vl' [-Wunused-command-line-argument]     # or an error if we make mavx512vl TargetSpecific

The -arch arm64 -mavx512vl situation isn't very common. Though not-carefully-written CMake may make the mistakes: llvm/lib/Support/BLAKE3/CMakeLists.txt (fixed by f231829304d15dca0dad8bafe35c4277904db602) and in-tree misuse: tsan (https://github.com/llvm/llvm-project/issues/63270).
(I think the two use sites are unusual.)

clang/include/clang/Driver/Options.h

enum ClangFlags {
   NoDriverOption = (1 << 12),
   LinkOption = (1 << 13),
   FlangOption = (1 << 14),
   FC1Option = (1 << 15),
   FlangOnlyOption = (1 << 16),
   DXCOption = (1 << 17),
   CLDXCOption = (1 << 18),
   Ignored = (1 << 19),
+  TargetSpecific = (1 << 20),
 };
 
 enum ID {
     OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
                HELPTEXT, METAVAR, VALUES)                                      \
   OPT_##ID,
 #include "clang/Driver/Options.inc"

clang/include/clang/Driver/Options.td

 
 // FlangOnlyOption - This option should only be used by Flang (i.e. it is not
 // available for Clang)
 def FlangOnlyOption : OptionFlag;
 
 // FC1Option - This option should be accepted by flang -fc1.
 def FC1Option : OptionFlag;
 
+// This is a target-specific option for compilation. Using it on an unsupported
+// target will lead to an err_drv_unsupported_opt_for_target error.
+def TargetSpecific : OptionFlag;
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName<string name> { string DocName = name; }
 
 // A brief description to show in documentation, interpreted as rST.
 class DocBrief<code descr> { code DocBrief = descr; }
 
 // Indicates that this group should be flattened into its parent when generating
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
 // Indicates that this warning is ignored, but accepted with a warning for
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
 /////////
 // Groups
 
 def Action_Group : OptionGroup<"<action group>">, DocName<"Actions">,
                    DocBrief<[{The action to perform on the input.}]>;
 
 // Meta-group for options which are only used for compilation,
 // and not linking etc.

 def fvisibility_global_new_delete_hidden : Flag<["-"], "fvisibility-global-new-delete-hidden">, Group<f_Group>,
   HelpText<"Give global C++ operator new and delete declarations hidden visibility">, Flags<[CC1Option]>,
   MarshallingInfoFlag<LangOpts<"GlobalAllocationFunctionVisibilityHidden">>;
 def mdefault_visibility_export_mapping_EQ : Joined<["-"], "mdefault-visibility-export-mapping=">,
   Values<"none,explicit,all">,
   NormalizedValuesScope<"LangOptions::DefaultVisiblityExportMapping">,
   NormalizedValues<["None", "Explicit", "All"]>,
   HelpText<"Mapping between default visibility and export">,
-  Group<m_Group>, Flags<[CC1Option]>,
+  Group<m_Group>, Flags<[CC1Option,TargetSpecific]>,
   MarshallingInfoEnum<LangOpts<"DefaultVisibilityExportMapping">,"None">;
 defm new_infallible : BoolFOption<"new-infallible",
   LangOpts<"NewInfallible">, DefaultFalse,
   PosFlag<SetTrue, [], "Enable">, NegFlag<SetFalse, [], "Disable">,
   BothFlags<[CC1Option], " treating throwing global C++ operator new as always returning valid memory "
   "(annotates with __attribute__((returns_nonnull)) and throw()). This is detectable in source.">>;
 defm whole_program_vtables : BoolFOption<"whole-program-vtables",
   CodeGenOpts<"WholeProgramVTables">, DefaultFalse,

 def mno_pure_code : Flag<["-"], "mno-pure-code">, Alias<mno_execute_only>;
 def mtvos_version_min_EQ : Joined<["-"], "mtvos-version-min=">, Group<m_Group>;
 def mappletvos_version_min_EQ : Joined<["-"], "mappletvos-version-min=">, Alias<mtvos_version_min_EQ>;
 def mtvos_simulator_version_min_EQ : Joined<["-"], "mtvos-simulator-version-min=">;
 def mappletvsimulator_version_min_EQ : Joined<["-"], "mappletvsimulator-version-min=">, Alias<mtvos_simulator_version_min_EQ>;
 def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group<m_Group>;
 def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">;
 def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias<mwatchos_simulator_version_min_EQ>;
-def march_EQ : Joined<["-"], "march=">, Group<m_Group>, Flags<[CoreOption]>,
+def march_EQ : Joined<["-"], "march=">, Group<m_Group>, Flags<[CoreOption,TargetSpecific]>,
   HelpText<"For a list of available architectures for the target use '-mcpu=help'">;
 def masm_EQ : Joined<["-"], "masm=">, Group<m_Group>, Flags<[NoXarchOption]>;
 def inline_asm_EQ : Joined<["-"], "inline-asm=">, Group<m_Group>, Flags<[CC1Option]>,
   Values<"att,intel">,
   NormalizedValuesScope<"CodeGenOptions">, NormalizedValues<["IAD_ATT", "IAD_Intel"]>,
   MarshallingInfoEnum<CodeGenOpts<"InlineAsmDialect">, "IAD_ATT">;
 def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group<m_Group>, Flags<[CC1Option]>,
   MarshallingInfoString<TargetOpts<"CodeModel">, [{"default"}]>;
 def mtls_size_EQ : Joined<["-"], "mtls-size=">, Group<m_Group>, Flags<[NoXarchOption, CC1Option]>,
   HelpText<"Specify bit size of immediate TLS offsets (AArch64 ELF only): "
            "12 (for 4KB) | 24 (for 16MB, default) | 32 (for 4GB) | 48 (for 256TB, needs -mcmodel=large)">,
   MarshallingInfoInt<CodeGenOpts<"TLSSize">>;
 def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group<m_Group>;
 def mdefault_build_attributes : Joined<["-"], "mdefault-build-attributes">, Group<m_Group>;
 def mno_default_build_attributes : Joined<["-"], "mno-default-build-attributes">, Group<m_Group>;
 def mconstant_cfstrings : Flag<["-"], "mconstant-cfstrings">, Group<clang_ignored_m_Group>;
 def mconsole : Joined<["-"], "mconsole">, Group<m_Group>, Flags<[NoXarchOption]>;
 def mwindows : Joined<["-"], "mwindows">, Group<m_Group>, Flags<[NoXarchOption]>;
 def mdll : Joined<["-"], "mdll">, Group<m_Group>, Flags<[NoXarchOption]>;
 def municode : Joined<["-"], "municode">, Group<m_Group>, Flags<[NoXarchOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group<m_Group>, Flags<[NoXarchOption]>;
 def mguard_EQ : Joined<["-"], "mguard=">, Group<m_Group>, Flags<[NoXarchOption]>,
   HelpText<"Enable or disable Control Flow Guard checks and guard tables emission">,
   Values<"none,cf,cf-nochecks">;
-def mcpu_EQ : Joined<["-"], "mcpu=">, Group<m_Group>, 
+def mcpu_EQ : Joined<["-"], "mcpu=">, Group<m_Group>, TargetSpecific,
   HelpText<"For a list of available CPUs for the target use '-mcpu=help'">;
 def mmcu_EQ : Joined<["-"], "mmcu=">, Group<m_Group>;
 def msim : Flag<["-"], "msim">, Group<m_Group>;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group<m_Group>;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group<clang_ignored_m_Group>;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group<clang_ignored_m_Group>;
 def minline_all_stringops : Flag<["-"], "minline-all-stringops">, Group<clang_ignored_m_Group>;
 def mno_inline_all_stringops : Flag<["-"], "mno-inline-all-stringops">, Group<clang_ignored_m_Group>;
defm zvector : BoolFOption<"zvector",
   NegFlag<SetFalse>>;
 def mzvector : Flag<["-"], "mzvector">, Alias<fzvector>;
 def mno_zvector : Flag<["-"], "mno-zvector">, Alias<fno_zvector>;
 
 def mxcoff_build_id_EQ : Joined<["-"], "mxcoff-build-id=">, Group<Link_Group>, MetaVarName<"<0xHEXSTRING>">,
   HelpText<"On AIX, request creation of a build-id string, \"0xHEXSTRING\", in the string table of the loader section inside the linked binary">;
 def mignore_xcoff_visibility : Flag<["-"], "mignore-xcoff-visibility">, Group<m_Group>,
 HelpText<"Not emit the visibility attribute for asm in AIX OS or give all symbols 'unspecified' visibility in XCOFF object file">,
-  Flags<[CC1Option]>;
+  Flags<[CC1Option,TargetSpecific]>;
 defm backchain : BoolOption<"m", "backchain",
   CodeGenOpts<"Backchain">, DefaultFalse,
   PosFlag<SetTrue, [], "Link stack frames through backchain on System Z">,
   NegFlag<SetFalse>, BothFlags<[NoXarchOption,CC1Option]>>, Group<m_Group>;
 
 def mno_warn_nonportable_cfstrings : Flag<["-"], "mno-warn-nonportable-cfstrings">, Group<m_Group>;
 def mno_omit_leaf_frame_pointer : Flag<["-"], "mno-omit-leaf-frame-pointer">, Group<m_Group>;
 def momit_leaf_frame_pointer : Flag<["-"], "momit-leaf-frame-pointer">, Group<m_Group>,

clang/lib/Driver/Driver.cpp

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 (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) {
+        if (A->getOption().hasFlag(options::TargetSpecific)) {
+          Diag(diag::err_drv_unsupported_opt_for_target)
+              << A->getSpelling() << getTargetTriple();
+        } else {
           Diag(clang::diag::warn_drv_unused_argument)
               << A->getAsString(C.getArgs());
         }
       }
     }
+  }
+}
 
 namespace {
 /// Utility class to control the collapse of dependent actions and select the
 /// tools accordingly.
 class ToolSelector final {
   /// The tool chain this selector refers to.
   const ToolChain &TC;
 

clang/lib/Driver/ToolChains/AIX.h

public:
 
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
 
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
 
+  void addClangTargetOptions(
+      const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CC1Args,
+      Action::OffloadKind DeviceOffloadingKind) const override;
+
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs) const override;
 
   CXXStdlibType GetDefaultCXXStdlibType() const override;
 
   RuntimeLibType GetDefaultRuntimeLibType() const override;
 
   // Set default DWARF version to 3 for now as latest AIX OS supports version 3.

clang/lib/Driver/ToolChains/AIX.cpp

 
 ToolChain::RuntimeLibType AIX::GetDefaultRuntimeLibType() const {
   return ToolChain::RLT_CompilerRT;
 }
 
 auto AIX::buildAssembler() const -> Tool * { return new aix::Assembler(*this); }
 
 auto AIX::buildLinker() const -> Tool * { return new aix::Linker(*this); }
+
+void AIX::addClangTargetOptions(
+    const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CC1Args,
+    Action::OffloadKind DeviceOffloadingKind) const {
+  Args.AddLastArg(CC1Args, options::OPT_mignore_xcoff_visibility);
+  Args.AddLastArg(CC1Args, options::OPT_mdefault_visibility_export_mapping_EQ);
+
+  if (Args.hasFlag(options::OPT_fxl_pragma_pack,
+                   options::OPT_fno_xl_pragma_pack, true))
+    CC1Args.push_back("-fxl-pragma-pack");
+}

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

std::string ppc::getPPCTuneCPU(const ArgList &Args, const llvm::Triple &T) {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_mtune_EQ))
     return normalizeCPUName(A->getValue(), T);
   return getPPCGenericTargetCPU(T);
 }
 
 /// Get the (LLVM) name of the PowerPC cpu we are targeting.
 std::string ppc::getPPCTargetCPU(const Driver &D, const ArgList &Args,
                                  const llvm::Triple &T) {
-  if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-    D.Diag(diag::err_drv_unsupported_opt_for_target)
-        << A->getSpelling() << T.getTriple();
-  }
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_mcpu_EQ))
     return normalizeCPUName(A->getValue(), T);
   return getPPCGenericTargetCPU(T);
 }
 
 const char *ppc::getPPCAsmModeForCPU(StringRef Name) {
   return llvm::StringSwitch<const char *>(Name)
       .Case("pwr7", "-mpower7")

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

if (ABI == sparc::FloatABI::Invalid) {
     ABI = sparc::FloatABI::Hard;
   }
 
   return ABI;
 }
 
 std::string sparc::getSparcTargetCPU(const Driver &D, const ArgList &Args,
                                      const llvm::Triple &Triple) {
-  if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-    D.Diag(diag::err_drv_unsupported_opt_for_target)
-        << A->getSpelling() << Triple.getTriple();
-    return "";
-  }
-
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mcpu_EQ)) {
     StringRef CPUName = A->getValue();
     if (CPUName == "native") {
       std::string CPU = std::string(llvm::sys::getHostCPUName());
       if (!CPU.empty() && CPU != "generic")
         return CPU;
       return "";
     }

clang/lib/Driver/ToolChains/Clang.cpp

if (const Arg *A =
         Args.AddLastArg(CmdArgs, options::OPT_fvisibility_nodllstorageclass_EQ);
         Args.AddLastArg(CmdArgs, options::OPT_fvisibility_externs_dllimport_EQ);
         Args.AddLastArg(CmdArgs,
                         options::OPT_fvisibility_externs_nodllstorageclass_EQ);
       }
     }
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_mignore_xcoff_visibility)) {
-    if (Triple.isOSAIX())
-      CmdArgs.push_back("-mignore-xcoff-visibility");
-    else
-      D.Diag(diag::err_drv_unsupported_opt_for_target)
-          << A->getAsString(Args) << TripleStr;
-  }
-
-  if (const Arg *A =
-          Args.getLastArg(options::OPT_mdefault_visibility_export_mapping_EQ)) {
-    if (Triple.isOSAIX())
-      A->render(Args, CmdArgs);
-    else
-      D.Diag(diag::err_drv_unsupported_opt_for_target)
-          << A->getAsString(Args) << TripleStr;
-  }
-
   if (Args.hasFlag(options::OPT_fvisibility_inlines_hidden,
                     options::OPT_fno_visibility_inlines_hidden, false))
     CmdArgs.push_back("-fvisibility-inlines-hidden");
 
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden_static_local_var,
                            options::OPT_fno_visibility_inlines_hidden_static_local_var);
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_global_new_delete_hidden);
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
if (A->getOption().matches(options::OPT_fdollars_in_identifiers))
       CmdArgs.push_back("-fdollars-in-identifiers");
     else
       CmdArgs.push_back("-fno-dollars-in-identifiers");
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fapple_pragma_pack,
                     options::OPT_fno_apple_pragma_pack);
 
-  if (Args.hasFlag(options::OPT_fxl_pragma_pack,
-                   options::OPT_fno_xl_pragma_pack, RawTriple.isOSAIX()))
-    CmdArgs.push_back("-fxl-pragma-pack");
-
   // Remarks can be enabled with any of the `-f.*optimization-record.*` flags.
   if (willEmitRemarks(Args) && checkRemarksOptions(D, Args, Triple))
     renderRemarksOptions(Args, CmdArgs, Triple, Input, Output, JA);
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
                                      options::OPT_fno_rewrite_imports, false);
   if (RewriteImports)
     CmdArgs.push_back("-frewrite-imports");