-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Clang][Driver] Enable offloadlib option for clang-cl #162980
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: jinge90 <[email protected]>
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: None (jinge90) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162980.diff 1 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index ec38231f906eb..4479aadf6949b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5970,10 +5970,10 @@ def : Flag<["-"], "nocudainc">, Alias<no_offload_inc>;
def no_offloadlib
: Flag<["--"], "no-offloadlib">,
MarshallingInfoFlag<LangOpts<"NoGPULib">>,
- Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
- HelpText<"Do not link device library for CUDA/HIP device compilation">;
+ Visibility<[ClangOption, CC1Option, CLOption, FlangOption, FC1Option]>,
+ HelpText<"Do not link device library for CUDA/HIP/SYCL device compilation">;
def offloadlib : Flag<["--"], "offloadlib">,
- Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
+ Visibility<[ClangOption, CC1Option, CLOption, FlangOption, FC1Option]>,
HelpText<"Link device libraries for GPU device compilation">;
def : Flag<["-"], "nogpulib">,
Alias<no_offloadlib>,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jinge90, I don't think the title accurately describes what the patch does.
I see that you enabled --[no-]offloadlib
flags for MSVC compatible driver. The only SYCL related change is the help message. Did you intend to put more changes in this PR?
Please, add a test for using --[no-]offloadlib
in CL compatibility mode.
Please update the PR description to provide essential context and details about the changes proposed in this PR. Thanks! |
Signed-off-by: jinge90 <[email protected]>
Hi, @bader Thanks very much. |
Hi, @srividya-sundaram |
If I get it right,
Please, capitalize SYCL in the description: sycl -> SYCL. |
Hi, @bader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
@srividya-sundaram, are you okay with this PR? |
Yes, the changes look good to me. |
// DEFAULT: "-sycl-std=2020" | ||
|
||
// RUN: %clang_cl -### -fsycl -sycl-std=2017 --no-offloadlib -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OFFLOADLIB | ||
// CHECK-NO-OFFLOADLIB-NOT: warning: unknown argument ignored in clang-cl: '--no-offloadlib' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably missed, but is there a corresponding test for clang driver as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @srividya-sundaram
clang and clangxx driver supports this option even without this PR, I add corresponding test to cover clang/clangxx driver to align with other tests in clang/test/Driver/sycl.c.
Thanks very much.
Also, just FYI, I don't seem to have approval permission. |
Signed-off-by: jinge90 <[email protected]>
--[no-]offloadlib option is used by rocm and cuda toolchain to enable/disable device libraries in linking phase for device code. It makes sense to re-use this option in SYCL for similar purpose and since clang driver supports SYCL in CL compatibility mode, we also need to enable this option in CL compatibility mode.