[clang][nfc] Split getOrCheckAMDGPUCodeObjectVersion

[JonChesterfield]

Summary

[clang][nfc] Split getOrCheckAMDGPUCodeObjectVersion

Separates detection of deprecated or invalid code object version from
returning the version. Written to avoid any behaviour change.

Precursor to a revision of D98746.

[JonChesterfield]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1610

This would probably be more useful if it diagnosed an invalid argument even when overridden by a legacy option. This patch preserves the existing behaviour.

1623

getAsInteger writes to CodeObjVer.

[yaxunl]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1605–1611

can we just call getAMDGPUCodeObjectVersion here instead of duplicating the logic?

1623

can we return Optional<unsigned> and return None if there is a remnant? Then we can call this function in checkAMDGPUCodeObjectVersion

[JonChesterfield]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1623

We could. It would spawn handling of the Optional<> at all the call sites though. I'll see if I can pull the search logic into a local helper.

[JonChesterfield]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1623

Implemented in D101117.

[JonChesterfield]

• factor out last arg wins

[JonChesterfield]

• whitespace

[JonChesterfield]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1576–1586

The logic to pick which of multiple arguments wins seems an important thing to keep in one place. I think this is an improvement on the first draft and on D101117

[yaxunl]

LGTM. Thanks.

clang/lib/Driver/ToolChains/AMDGPU.cpp

AMDGPUToolChain::AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
                                  const ArgList &Args)
     : Generic_ELF(D, Triple, Args),
       OptionsDefault(
           {{options::OPT_O, "3"}, {options::OPT_cl_std_EQ, "CL1.2"}}) {
   // Check code object version options. Emit warnings for legacy options
   // and errors for the last invalid code object version options.
   // It is done here to avoid repeated warning or error messages for
   // each tool invocation.
-  (void)getOrCheckAMDGPUCodeObjectVersion(D, Args, /*Diagnose=*/true);
+  checkAMDGPUCodeObjectVersion(D, Args);
 }
 
 Tool *AMDGPUToolChain::buildLinker() const {
   return new tools::amdgpu::Linker(*this);
 }
 
 DerivedArgList *
 AMDGPUToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,

clang/lib/Driver/ToolChains/Clang.cpp

static const char *RelocationModelName(llvm::Reloc::Model Model) {
   case llvm::Reloc::ROPI_RWPI:
     return "ropi-rwpi";
   }
   llvm_unreachable("Unknown Reloc::Model kind");
 }
 static void handleAMDGPUCodeObjectVersionOptions(const Driver &D,
                                                  const ArgList &Args,
                                                  ArgStringList &CmdArgs) {
-  unsigned CodeObjVer = getOrCheckAMDGPUCodeObjectVersion(D, Args);
+  unsigned CodeObjVer = getAMDGPUCodeObjectVersion(D, Args);
   CmdArgs.insert(CmdArgs.begin() + 1,
                  Args.MakeArgString(Twine("--amdhsa-code-object-version=") +
                                     Twine(CodeObjVer)));
   CmdArgs.insert(CmdArgs.begin() + 1, "-mllvm");
 }
 
 void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
                                     const Driver &D, const ArgList &Args,

clang/lib/Driver/ToolChains/CommonArgs.h

 /// \p Flag must be a flag accepted by the driver with its leading '-' removed,
 //     otherwise '-print-multi-lib' will not emit them correctly.
 void addMultilibFlag(bool Enabled, const char *const Flag,
                      Multilib::flags_list &Flags);
 
 void addX86AlignBranchArgs(const Driver &D, const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs, bool IsLTO);
 
-unsigned getOrCheckAMDGPUCodeObjectVersion(const Driver &D,
-                                           const llvm::opt::ArgList &Args,
-                                           bool Diagnose = false);
+void checkAMDGPUCodeObjectVersion(const Driver &D,
+                                  const llvm::opt::ArgList &Args);
+
+unsigned getAMDGPUCodeObjectVersion(const Driver &D,
+                                    const llvm::opt::ArgList &Args);
 
 void addMachineOutlinerArgs(const Driver &D, const llvm::opt::ArgList &Args,
                             llvm::opt::ArgStringList &CmdArgs,
                             const llvm::Triple &Triple, bool IsLTO);
 
 void addOpenMPDeviceRTL(const Driver &D, const llvm::opt::ArgList &DriverArgs,
                         llvm::opt::ArgStringList &CC1Args,
                         StringRef BitcodeSuffix, const llvm::Triple &Triple);
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONARGS_H

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (Value.getAsInteger(10, PrefixSize)) {
       D.Diag(diag::err_drv_invalid_argument_to_option)
           << Value << A->getOption().getName();
     } else {
       addArg("-x86-pad-max-prefix-size=" + Twine(PrefixSize));
     }
   }
 }
 
-unsigned tools::getOrCheckAMDGPUCodeObjectVersion(
-    const Driver &D, const llvm::opt::ArgList &Args, bool Diagnose) {
+static llvm::opt::Arg *
+getAMDGPUCodeObjectArgument(const Driver &D, const llvm::opt::ArgList &Args) {
+  // The last of -mcode-object-v3, -mno-code-object-v3 and
+  // -mcode-object-version=<version> wins.
+  return Args.getLastArg(options::OPT_mcode_object_v3_legacy,
+                         options::OPT_mno_code_object_v3_legacy,
+                         options::OPT_mcode_object_version_EQ);
+}
+
+void tools::checkAMDGPUCodeObjectVersion(const Driver &D,
+                                         const llvm::opt::ArgList &Args) {
   const unsigned MinCodeObjVer = 2;
   const unsigned MaxCodeObjVer = 4;
-  unsigned CodeObjVer = 4;
 
   // Emit warnings for legacy options even if they are overridden.
-  if (Diagnose) {
-    if (Args.hasArg(options::OPT_mno_code_object_v3_legacy))
-      D.Diag(diag::warn_drv_deprecated_arg) << "-mno-code-object-v3"
-                                            << "-mcode-object-version=2";
 
-    if (Args.hasArg(options::OPT_mcode_object_v3_legacy))
-      D.Diag(diag::warn_drv_deprecated_arg) << "-mcode-object-v3"
-                                            << "-mcode-object-version=3";
-  }
+
+  if (auto *CodeObjArg = getAMDGPUCodeObjectArgument(D, Args)) {
+    if (CodeObjArg->getOption().getID() ==
+        options::OPT_mcode_object_version_EQ) {
+      unsigned CodeObjVer = MaxCodeObjVer;
+      auto Remnant =
+          StringRef(CodeObjArg->getValue()).getAsInteger(0, CodeObjVer);
+      if (Remnant || CodeObjVer < MinCodeObjVer || CodeObjVer > MaxCodeObjVer)
+        D.Diag(diag::err_drv_invalid_int_value)
+            << CodeObjArg->getAsString(Args) << CodeObjArg->getValue();
+    }
+  }
 
-  // The last of -mcode-object-v3, -mno-code-object-v3 and
-  // -mcode-object-version=<version> wins.
-  if (auto *CodeObjArg =
-          Args.getLastArg(options::OPT_mcode_object_v3_legacy,
-                          options::OPT_mno_code_object_v3_legacy,
-                          options::OPT_mcode_object_version_EQ)) {
+unsigned tools::getAMDGPUCodeObjectVersion(const Driver &D,
+                                           const llvm::opt::ArgList &Args) {
+  unsigned CodeObjVer = 4; // default
+  if (auto *CodeObjArg = getAMDGPUCodeObjectArgument(D, Args)) {
     if (CodeObjArg->getOption().getID() ==
         options::OPT_mno_code_object_v3_legacy) {
       CodeObjVer = 2;
     } else if (CodeObjArg->getOption().getID() ==
                options::OPT_mcode_object_v3_legacy) {
       CodeObjVer = 3;
     } else {
-      auto Remnant =
-          StringRef(CodeObjArg->getValue()).getAsInteger(0, CodeObjVer);
-      if (Diagnose &&
-          (Remnant || CodeObjVer < MinCodeObjVer || CodeObjVer > MaxCodeObjVer))
-        D.Diag(diag::err_drv_invalid_int_value)
-            << CodeObjArg->getAsString(Args) << CodeObjArg->getValue();
     }
   }
   return CodeObjVer;
 }
 
 void tools::addMachineOutlinerArgs(const Driver &D,
                                    const llvm::opt::ArgList &Args,
                                    llvm::opt::ArgStringList &CmdArgs,

clang/lib/Driver/ToolChains/HIP.cpp

void AMDGCN::constructHIPFatbinCommand(Compilation &C, const JobAction &JA,
   // clang-offload-bundler.
   std::string BundlerTargetArg = "-targets=host-x86_64-unknown-linux";
   std::string BundlerInputArg = "-inputs=" NULL_FILE;
 
   // For code object version 2 and 3, the offload kind in bundle ID is 'hip'
   // for backward compatibility. For code object version 4 and greater, the
   // offload kind in bundle ID is 'hipv4'.
   std::string OffloadKind = "hip";
-  if (getOrCheckAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4)
+  if (getAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4)
     OffloadKind = OffloadKind + "v4";
   for (const auto &II : Inputs) {
     const auto* A = II.getAction();
     BundlerTargetArg = BundlerTargetArg + "," + OffloadKind +
                        "-amdgcn-amd-amdhsa--" +
                        StringRef(A->getOffloadingArch()).str();
     BundlerInputArg = BundlerInputArg + "," + II.getFilename();
   }