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

[HIP] Claim --offload-compress for -M #133456

Merged
merged 1 commit into from
Apr 2, 2025

Conversation

yxsamliu
Copy link
Collaborator

Cmake automatically generates dependency files with all compilation options provided by users. When users use --offload-compress for HIP compilation, it causes warnings when cmake generates dependency files. Claim this option to suppress warnings.

@yxsamliu yxsamliu requested review from MaskRay, Artem-B and jhuber6 March 28, 2025 15:34
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-clang

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

Cmake automatically generates dependency files with all compilation options provided by users. When users use --offload-compress for HIP compilation, it causes warnings when cmake generates dependency files. Claim this option to suppress warnings.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+6)
  • (modified) clang/test/Driver/hip-options.hip (+6)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 40ecc0aee48b3..47c8bd31f0881 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1026,6 +1026,12 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
       CmdArgs.push_back("-dependency-file");
       CmdArgs.push_back(DepFile);
     }
+    // Cmake generates dependency files using all compilation options specified
+    // by users. Claim those not used for dependency files.
+    if (JA.isOffloading(Action::OFK_HIP)) {
+      Args.ClaimAllArgs(options::OPT_offload_compress);
+      Args.ClaimAllArgs(options::OPT_no_offload_compress);
+    }
 
     bool HasTarget = false;
     for (const Arg *A : Args.filtered(options::OPT_MT, options::OPT_MQ)) {
diff --git a/clang/test/Driver/hip-options.hip b/clang/test/Driver/hip-options.hip
index 0aabc8ad41904..962676ada9497 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -242,3 +242,9 @@
 // NO-WARN-ATOMIC: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-Werror=atomic-alignment" {{.*}} "-Wno-error=atomic-alignment"
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Werror=atomic-alignment"
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Wno-error=atomic-alignment"
+
+// Check --offload-compress does not cause warning.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx1100 --offload-compress --offload-host-only -M %s \
+// RUN:   2>&1 | FileCheck -check-prefix=COMPRESS %s
+// COMPRESS-NOT: clang: warning: argument unused during compilation: '--offload-compress'

@yxsamliu yxsamliu changed the title [HIP] Claim --offloading-compress for -M [HIP] Claim --offload-compress for -M Mar 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-clang-driver

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

Cmake automatically generates dependency files with all compilation options provided by users. When users use --offload-compress for HIP compilation, it causes warnings when cmake generates dependency files. Claim this option to suppress warnings.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+6)
  • (modified) clang/test/Driver/hip-options.hip (+6)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 40ecc0aee48b3..47c8bd31f0881 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1026,6 +1026,12 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
       CmdArgs.push_back("-dependency-file");
       CmdArgs.push_back(DepFile);
     }
+    // Cmake generates dependency files using all compilation options specified
+    // by users. Claim those not used for dependency files.
+    if (JA.isOffloading(Action::OFK_HIP)) {
+      Args.ClaimAllArgs(options::OPT_offload_compress);
+      Args.ClaimAllArgs(options::OPT_no_offload_compress);
+    }
 
     bool HasTarget = false;
     for (const Arg *A : Args.filtered(options::OPT_MT, options::OPT_MQ)) {
diff --git a/clang/test/Driver/hip-options.hip b/clang/test/Driver/hip-options.hip
index 0aabc8ad41904..962676ada9497 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -242,3 +242,9 @@
 // NO-WARN-ATOMIC: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-Werror=atomic-alignment" {{.*}} "-Wno-error=atomic-alignment"
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Werror=atomic-alignment"
 // NO-WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-Wno-error=atomic-alignment"
+
+// Check --offload-compress does not cause warning.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx1100 --offload-compress --offload-host-only -M %s \
+// RUN:   2>&1 | FileCheck -check-prefix=COMPRESS %s
+// COMPRESS-NOT: clang: warning: argument unused during compilation: '--offload-compress'

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this only cause warnings when it makes dependency files? If we can check whether or not it's HIP, then why isn't any of the HIP argument parsing happening?

@yxsamliu
Copy link
Collaborator Author

yxsamliu commented Mar 28, 2025

Why does this only cause warnings when it makes dependency files?

Because in normal HIP compilations (compilations not for generating dependency files), --offloading-compress is parsed by offload bundler and offload wrapper actions when creating job actions. However, there are no such actions to parse them in the compilation that only generates dependency files.

If we can check whether or not it's HIP, then why isn't any of the HIP argument parsing happening?

Sorry I did not quite understand this question. Can you elaborate? Thanks.

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 28, 2025

Why does this only cause warnings when it makes dependency files?

Because --offloading-compress is parsed by offload bundler and offload wrapper actions when creating job actions. There is no action to parse them in the compilation that only generates dependency files.

If we can check whether or not it's HIP, then why isn't any of the HIP argument parsing happening?

Sorry I did not quite understand this question. Can you elaborate? Thanks.

I just feel like this should probably go in the HIP toolchain when it translates arguments or something instead of just clang.

@yxsamliu
Copy link
Collaborator Author

Why does this only cause warnings when it makes dependency files?

Because --offloading-compress is parsed by offload bundler and offload wrapper actions when creating job actions. There is no action to parse them in the compilation that only generates dependency files.

If we can check whether or not it's HIP, then why isn't any of the HIP argument parsing happening?

Sorry I did not quite understand this question. Can you elaborate? Thanks.

I just feel like this should probably go in the HIP toolchain when it translates arguments or something instead of just clang.

In the compilation that generates the dependency files for HIP, --offload-host-only is used. The argument translation is done by the host toolchain. It may not be convenient to update all the host toolchains about HIP.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't notice the indentation, LGTM.

Cmake automatically generate dependency files with all compilation
options provided by users. When users use  for
HIP compilation, it causes warnings when cmake generates dependency
files. Claim this option to supress warnings.
@yxsamliu yxsamliu force-pushed the claim-offload-compress branch from 674c4ef to be4ea80 Compare April 2, 2025 12:33
@yxsamliu yxsamliu merged commit dedb632 into llvm:main Apr 2, 2025
11 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.

None yet

4 participants