[HIP] compile option code-object-v3 propagate to llc

[ashi1]

Summary

This patch allows the compile option -mcode-object-v3 to be set by default, unless the -mno-code-object-v3 is added to the compile options.

[kzhuravl]

Can you handle all features as follows instead of checking a particular option?

handleTargetFeaturesGroup(Args, Features, options::OPT_m_amdgpu_Features_Group)

There is code in AMDGPU tool chain you can use as a reference.

[kzhuravl]

https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/AMDGPU.cpp#L52

[ashi1]

Changed patch to us handleTargetFeatures, to consume all -m options passed into clang.

[kzhuravl]

Can you add some tests? Preferably with -mxnack/-mno-xnack, -msram-ecc/-mno-sram-ecc, -mcode-object-v3/-mno-code-object-v3

[ashi1]

I've added a lit test to check for options such as -mxnack/-mno-xnack, -msram-ecc/-mno-sram-ecc, -mcode-object-v3/-mno-code-object-v3

[kzhuravl]

Is it still code object v2 by default?

[ashi1]

Yes, but it will need this PR: https://github.com/ROCm-Developer-Tools/HIP/pull/910
So that the default is set.

[kzhuravl]

LGTM. Please make sure to update your commit message as your patch processes all m options now.

[arsenm]

I think this breaks the attributes already listed in the functions on the IR since -mattr overrides those. I think we need to stop using subtarget features to communicate this

[rnk]

I reverted this in rC353893 because it was still causing test failures.

[ashi1]

This was submitted again with the fix. Thanks!

lib/Driver/ToolChains/HIP.cpp

     Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
     const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
     llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
   // Construct llc command.
   // FIXME: -disable-promote-alloca-to-lds is a workaround for issues in
   // AMDGPUPromoteAlloca pass which cause invalid memory access in PyTorch.
   // Remove this once the issue is fixed.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-                        "-filetype=obj", "-mattr=-code-object-v3",
+                        "-filetype=obj",
                         "-disable-promote-alloca-to-lds",
-                        Args.MakeArgString("-mcpu=" + SubArchName), "-o"};
+                        Args.MakeArgString("-mcpu=" + SubArchName)};
+
+  // Extract all the -m options
+  std::vector<llvm::StringRef> Features;
+  handleTargetFeaturesGroup(
+    Args, Features, options::OPT_m_amdgpu_Features_Group);
+
+  // Add features to mattr such as code-object-v3 and xnack
+  std::string MAttrString = "-mattr=";
+  for(auto OneFeature : Features) {
+    MAttrString.append(Args.MakeArgString(OneFeature));
+    if (OneFeature != Features.back())
+      MAttrString.append(",");
+  }
+  LlcArgs.push_back(Args.MakeArgString(MAttrString));
+
+  // Add output filename
+  LlcArgs.push_back("-o");
   std::string LlcOutputFileName =
       C.getDriver().GetTemporaryPath(OutputFilePrefix, "o");
   const char *LlcOutputFile =
       C.addTempFile(C.getArgs().MakeArgString(LlcOutputFileName));
   LlcArgs.push_back(LlcOutputFile);
   SmallString<128> LlcPath(C.getDriver().Dir);
   llvm::sys::path::append(LlcPath, "llc");
   const char *Llc = Args.MakeArgString(LlcPath);

test/Driver/hip-toolchain-features.hip

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mcode-object-v3 2>&1 | FileCheck %s -check-prefix=COV3
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-code-object-v3 2>&1 | FileCheck %s -check-prefix=NOCOV3
+
+// COV3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3"
+// NOCOV3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mxnack 2>&1 | FileCheck %s -check-prefix=XNACK
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-xnack 2>&1 | FileCheck %s -check-prefix=NOXNACK
+
+// XNACK: {{.*}}clang{{.*}}"-target-feature" "+xnack"
+// NOXNACK: {{.*}}clang{{.*}}"-target-feature" "-xnack"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -msram-ecc 2>&1 | FileCheck %s -check-prefix=SRAM
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-sram-ecc 2>&1 | FileCheck %s -check-prefix=NOSRAM
+
+// SRAM: {{.*}}clang{{.*}}"-target-feature" "+sram-ecc"
+// NOSRAM: {{.*}}clang{{.*}}"-target-feature" "-sram-ecc"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mcode-object-v3 -mxnack -msram-ecc \
+// RUN:   2>&1 | FileCheck %s -check-prefix=ALL3
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-code-object-v3 -mno-xnack -mno-sram-ecc \
+// RUN:   2>&1 | FileCheck %s -check-prefix=NOALL3
+
+// ALL3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3" "-target-feature" "+xnack" "-target-feature" "+sram-ecc"
+// NOALL3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3" "-target-feature" "-xnack" "-target-feature" "-sram-ecc"