Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NvlinkWrapper] Use -plugin-opt=mattr= instead of a custom feature #111712

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Oct 9, 2024

Summary:
We don't need a custom flag for this, LLVM had a way to get the features
which are forwarded via plugin-opt.

Summary:
We don't need a custom flag for this, LLVM had a way to get the features
which are forwarded via `plugin-opt`.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
We don't need a custom flag for this, LLVM had a way to get the features
which are forwarded via plugin-opt.


Full diff: https://github.com/llvm/llvm-project/pull/111712.diff

4 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Cuda.cpp (+2-2)
  • (modified) clang/test/Driver/cuda-cross-compiling.c (+1-1)
  • (modified) clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp (+1-1)
  • (modified) clang/tools/clang-nvlink-wrapper/NVLinkOpts.td (-3)
diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp
index 509cd87b28c37e..dfcd20a73f1d54 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -632,8 +632,8 @@ void NVPTX::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   std::vector<StringRef> Features;
   getNVPTXTargetFeatures(C.getDriver(), getToolChain().getTriple(), Args,
                          Features);
-  for (StringRef Feature : Features)
-    CmdArgs.append({"--feature", Args.MakeArgString(Feature)});
+  CmdArgs.push_back(
+      Args.MakeArgString("--plugin-opt=mattr=" + llvm::join(Features, ",")));
 
   // Add paths for the default clang library path.
   SmallString<256> DefaultLibPath =
diff --git a/clang/test/Driver/cuda-cross-compiling.c b/clang/test/Driver/cuda-cross-compiling.c
index 5f24e7a5accb08..54c291fac66ffd 100644
--- a/clang/test/Driver/cuda-cross-compiling.c
+++ b/clang/test/Driver/cuda-cross-compiling.c
@@ -104,4 +104,4 @@
 // RUN: %clang -target nvptx64-nvidia-cuda --cuda-feature=+ptx63 -march=sm_52 -### %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=FEATURE %s
 
-// FEATURE: clang-nvlink-wrapper{{.*}}"--feature" "+ptx63"
+// FEATURE: clang-nvlink-wrapper{{.*}}"--plugin-opt=mattr=+ptx63"
diff --git a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
index b4b376fe0d114e..b9767a7a03d0b5 100644
--- a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
+++ b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
@@ -344,7 +344,7 @@ Expected<std::unique_ptr<lto::LTO>> createLTO(const ArgList &Args) {
   Conf.RemarksHotnessThreshold = RemarksHotnessThreshold;
   Conf.RemarksFormat = RemarksFormat;
 
-  Conf.MAttrs = {Args.getLastArgValue(OPT_feature, "").str()};
+  Conf.MAttrs = llvm::codegen::getMAttrs();
   std::optional<CodeGenOptLevel> CGOptLevelOrNone =
       CodeGenOpt::parseLevel(Args.getLastArgValue(OPT_O, "2")[0]);
   assert(CGOptLevelOrNone && "Invalid optimization level");
diff --git a/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td b/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
index eeb9d1a6228240..a80c5937b42992 100644
--- a/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
+++ b/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
@@ -47,9 +47,6 @@ def arch : Separate<["--", "-"], "arch">,
 def : Joined<["--", "-"], "plugin-opt=mcpu=">,
   Flags<[HelpHidden, WrapperOnlyOption]>, Alias<arch>;
 
-def feature : Separate<["--", "-"], "feature">, Flags<[WrapperOnlyOption]>,
-  HelpText<"Specify the '+ptx' freature to use for LTO.">;
-
 def g : Flag<["-"], "g">, HelpText<"Specify that this was a debug compile.">;
 def debug : Flag<["--"], "debug">, Alias<g>;
 

@jhuber6 jhuber6 closed this Oct 14, 2024
@jhuber6 jhuber6 deleted the mattr branch October 14, 2024 19:20
@jhuber6 jhuber6 restored the mattr branch October 18, 2024 22:01
@jhuber6 jhuber6 reopened this Oct 18, 2024
@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 18, 2024

Ping

@jhuber6 jhuber6 merged commit 416731b into llvm:main Oct 18, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants