[Clang][AMDGPU] Set LTO CG opt level based on Clang option

[scott.linder]

Summary

For AMDGCN default to mapping --lto-O# to --lto-CGO# in a 1:1 manner
(i.e. clang -O<N> implies --lto-O<N> and --lto-CGO<N>).

Ensure there is a means to override this via -Xoffload-linker and begin
to claim these arguments to avoid incorrect warnings that they are not
used.

[scott.linder]

(Just adding everyone from the parent review, feel free to remove yourself as reviewer if you aren't interested!)

It isn't ideal, but as only AMDGCN wants the 1:1 mapping for LTO->CG opt level to be the default I implemented it here. Thoughts?

I can move the missing claim call to another patch, it just seemed like a small enough change to include inline.

[arsenm]

Making this target specific doesn’t make sense

[scott.linder]

In D142499#4078177, @arsenm wrote:

Making this target specific doesn’t make sense

I don't know how else to resolve others wanting the default to be the CGOptLevel = clamp(ltoOptLevel, 2, 3) behavior and AMDGPU wanting the default to be CGOptLevel = ltoOptLevel. Maybe that isn't actually the default we want? We can ask users to specify -Xoffload-linker --lto-CGO#.

[scott.linder]

Rebase

[scott.linder]

Rebase

[scott.linder]

Are there any thoughts on whether this is too ugly to live? It will be awkward to teach users the current default behavior without this change, but if we can accept it as a historical quirk that may be OK.

The primary driver for wanting the 1:1 mapping is an odd interaction between our debug info (only implemented for -O0 codegen currently, higher opt levels are WIP) and our implementation of -fgpu-rdc (currently this implies -flto as we don't support true device code-object linking).

A user who compiles and debugs without -fgpu-rdc cannot simply add it to their command-line, they will also need -Xoffload-linker --lto-CGO0. Of course, this is a temporary arrangement, but it also just seems "correct" to default to the 1:1 mapping. Even once our debug info supports higher opt levels, I suspect users will still explicitly request no optimization and be surprised that some arcane pass-thru option like -Xoffload-linker --lto-CGO0 is required to make it work how they expect.

[MaskRay]

Fine with me, but you may need an AMDGPU reviewer.

[yaxunl]

LGTM. Thanks

clang/lib/Driver/ToolChains/CommonArgs.cpp

 bool tools::isUseSeparateSections(const llvm::Triple &Triple) {
   return Triple.getOS() == llvm::Triple::CloudABI || Triple.isPS();
 }
 
 void tools::addLTOOptions(const ToolChain &ToolChain, const ArgList &Args,
                           ArgStringList &CmdArgs, const InputInfo &Output,
                           const InputInfo &Input, bool IsThinLTO) {
   const bool IsOSAIX = ToolChain.getTriple().isOSAIX();
+  const bool IsAMDGCN = ToolChain.getTriple().isAMDGCN();
   const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
   const Driver &D = ToolChain.getDriver();
   if (llvm::sys::path::filename(Linker) != "ld.lld" &&
       llvm::sys::path::stem(Linker) != "ld.lld") {
     // Tell the linker to load the plugin. This has to come before
     // AddLinkerInputs as gold requires -plugin and AIX ld requires -bplugin to
     // come before any -plugin-opt/-bplugin_opt that -Wl might forward.
     const char *PluginPrefix = IsOSAIX ? "-bplugin:" : "";
if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
     else if (A->getOption().matches(options::OPT_O)) {
       OOpt = A->getValue();
       if (OOpt == "g")
         OOpt = "1";
       else if (OOpt == "s" || OOpt == "z")
         OOpt = "2";
     } else if (A->getOption().matches(options::OPT_O0))
       OOpt = "0";
-    if (!OOpt.empty())
+    if (!OOpt.empty()) {
       CmdArgs.push_back(
           Args.MakeArgString(Twine(PluginOptPrefix) + ExtraDash + "O" + OOpt));
+      if (IsAMDGCN)
+        CmdArgs.push_back(Args.MakeArgString(Twine("--lto-CGO") + OOpt));
+    }
   }
 
   if (Args.hasArg(options::OPT_gsplit_dwarf))
     CmdArgs.push_back(Args.MakeArgString(
         Twine(PluginOptPrefix) + "dwo_dir=" + Output.getFilename() + "_dwo"));
 
   if (IsThinLTO)
     CmdArgs.push_back(Args.MakeArgString(Twine(PluginOptPrefix) + "thinlto"));

clang/lib/Driver/ToolChains/HIPAMD.cpp

LldArgs.push_back(
         Args.MakeArgString(Twine("-plugin-opt=") + A->getValue(0)));
   }
 
   if (C.getDriver().isSaveTempsEnabled())
     LldArgs.push_back("-save-temps");
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
-  for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker))
+  for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
     LldArgs.push_back(Arg->getValue(1));
+    Arg->claim();
+  }
 
   LldArgs.append({"-o", Output.getFilename()});
   for (auto Input : Inputs)
     LldArgs.push_back(Input.getFilename());
 
   // Look for archive of bundled bitcode in arguments, and add temporary files
   // for the extracted archive of bitcode to inputs.
   auto TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);

clang/test/Driver/hip-toolchain-opt.hip

 
 // RUN: %clang -### -Og \
 // RUN:   --target=x86_64-unknown-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -c -nogpulib \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck --check-prefixes=ALL,Og %s
 
+// RUN: %clang -### -O0 \
+// RUN:   -Xoffload-linker --lto-CGO2 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,O0-CGO2 %s
+
 // ALL: "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // DEFAULT-NOT: "-O{{.}}"
 // O0-SAME: "-O0"
 // O1-SAME: "-O1"
 // O2-SAME: "-O2"
 // O3-SAME: "-O3"
 // Os-SAME: "-Os"
 // Oz-SAME: "-Oz"
 // Og-SAME: "-Og"
+// O0-CGO2-SAME: "-O0"
+// O0-CGO2-NOT: "--lto-CGO2"
 
 // ALL-NOT: "{{.*}}opt"
 
 // ALL-NOT: "{{.*}}llc"
 
 // ALL: "{{.*}}lld{{.*}}" {{.*}} "-plugin-opt=mcpu=gfx900"
 // DEFAULT-NOT: "-plugin-opt=O{{.*}}"
 // O0-SAME: "-plugin-opt=O0"
+// O0-SAME: "--lto-CGO0"
 // O1-SAME: "-plugin-opt=O1"
+// O1-SAME: "--lto-CGO1"
 // O2-SAME: "-plugin-opt=O2"
+// O2-SAME: "--lto-CGO2"
 // O3-SAME: "-plugin-opt=O3"
+// O3-SAME: "--lto-CGO3"
 // Os-SAME: "-plugin-opt=O2"
+// Os-SAME: "--lto-CGO2"
 // Oz-SAME: "-plugin-opt=O2"
+// Oz-SAME: "--lto-CGO2"
 // Og-SAME: "-plugin-opt=O1"
+// Og-SAME: "--lto-CGO1"
+// O0-CGO2-SAME: "-plugin-opt=O0"
+// O0-CGO2-SAME: "--lto-CGO0"
+// O0-CGO2-SAME: "--lto-CGO2"
 
 // ALL: "-cc1" "-triple" "x86_64-unknown-linux-gnu"
 // DEFAULT-NOT: "-O{{.}}"
 // O0-SAME: "-O0"
 // O1-SAME: "-O1"
 // O2-SAME: "-O2"
 // O3-SAME: "-O3"
 // Os-SAME: "-Os"
 // Oz-SAME: "-Oz"
 // Og-SAME: "-Og"
+// O0-CGO2-SAME: "-O0"
+// O0-CGO2-NOT: "--lto-CGO2"