-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Clang][HIP] Warn when __AMDGCN_WAVEFRONT_SIZE is used in host code #91478
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /*===---- __clang_hip_device_macro_guards.h - guards for HIP device macros -=== | ||
| * | ||
| * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| * See https://llvm.org/LICENSE.txt for license information. | ||
| * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| * | ||
| *===-----------------------------------------------------------------------=== | ||
| */ | ||
|
|
||
| /* | ||
| * WARNING: This header is intended to be directly -include'd by | ||
| * the compiler and is not supposed to be included by users. | ||
| * | ||
| */ | ||
|
|
||
| #ifndef __CLANG_HIP_DEVICE_MACRO_GUARDS_H__ | ||
| #define __CLANG_HIP_DEVICE_MACRO_GUARDS_H__ | ||
|
|
||
| #if __HIP__ | ||
| #if !defined(__HIP_DEVICE_COMPILE__) | ||
| // The __AMDGCN_WAVEFRONT_SIZE macros cannot hold meaningful values during host | ||
| // compilation as devices are not initialized when the macros are defined and | ||
| // there may indeed be devices with differing wavefront sizes in the same | ||
| // system. This code issues diagnostics when the macros are used in host code. | ||
|
|
||
| #undef __AMDGCN_WAVEFRONT_SIZE | ||
| #undef __AMDGCN_WAVEFRONT_SIZE__ | ||
|
|
||
| // Reference __hip_device_macro_guard in a way that is legal in preprocessor | ||
| // directives and does not affect the value so that appropriate diagnostics are | ||
| // issued. Function calls, casts, or the comma operator would make the macro | ||
| // illegal for use in preprocessor directives. | ||
| #define __AMDGCN_WAVEFRONT_SIZE (!__hip_device_macro_guard ? 64 : 64) | ||
| #define __AMDGCN_WAVEFRONT_SIZE__ (!__hip_device_macro_guard ? 64 : 64) | ||
|
|
||
| // This function is referenced by the macro in device functions during host | ||
| // compilation, it SHOULD NOT cause a diagnostic. | ||
| __attribute__((device)) static constexpr int __hip_device_macro_guard(void) { | ||
| return -1; | ||
| } | ||
|
|
||
| // This function is referenced by the macro in host functions during host | ||
| // compilation, it SHOULD cause a diagnostic. | ||
| __attribute__(( | ||
| host, deprecated("The __AMDGCN_WAVEFRONT_SIZE macros do not correspond " | ||
| "to the device(s) when used in host code and may only " | ||
| "be used in device code."))) static constexpr int | ||
| __hip_device_macro_guard(void) { | ||
| return -1; | ||
| } | ||
| // TODO Change "deprecated" to "unavailable" to cause hard errors instead of | ||
| // warnings. | ||
| #endif | ||
| #endif // __HIP__ | ||
| #endif // __CLANG_HIP_DEVICE_MACRO_GUARDS_H__ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| // REQUIRES: amdgpu-registered-target | ||
| // RUN: %clang -xhip --offload-arch=gfx1030 --offload-host-only -pedantic -nogpuinc -nogpulib -nobuiltininc -nostdinc -fsyntax-only -Xclang -verify=onhost %s | ||
| // RUN: %clang -xhip --offload-arch=gfx1030 --offload-device-only -pedantic -nogpuinc -nogpulib -nobuiltininc -nostdinc -fsyntax-only -Xclang -verify=ondevice %s | ||
|
|
||
| // ondevice-no-diagnostics | ||
|
|
||
| #define WRAPPED __AMDGCN_WAVEFRONT_SIZE__ | ||
|
|
||
| __attribute__((host, device)) void use(int, const char*); | ||
|
|
||
| template<int N> __attribute__((host, device)) int templatify(int x) { | ||
| return x + N; | ||
| } | ||
|
|
||
| // no warning expected | ||
| #if defined(__HIP_DEVICE_COMPILE__) && (__AMDGCN_WAVEFRONT_SIZE__ == 64) && (__AMDGCN_WAVEFRONT_SIZE == 64) | ||
| int foo(void); | ||
| #endif | ||
|
|
||
| // no warning expected | ||
| __attribute__((device)) int device_var = __AMDGCN_WAVEFRONT_SIZE__; | ||
|
|
||
| __attribute__((device)) | ||
| void device_fun() { | ||
| // no warnings expected | ||
| use(__AMDGCN_WAVEFRONT_SIZE, "device function"); | ||
| use(__AMDGCN_WAVEFRONT_SIZE__, "device function"); | ||
| use(WRAPPED, "device function"); | ||
| use(templatify<__AMDGCN_WAVEFRONT_SIZE__>(42), "device function"); | ||
| } | ||
|
|
||
| // warning expected | ||
| int host_var = __AMDGCN_WAVEFRONT_SIZE__; // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
|
|
||
| __attribute__((host)) | ||
| void host_fun() { | ||
| // warnings expected | ||
| use(__AMDGCN_WAVEFRONT_SIZE, "host function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| use(__AMDGCN_WAVEFRONT_SIZE__, "host function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| use(WRAPPED, "host function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| use(templatify<__AMDGCN_WAVEFRONT_SIZE__>(42), "host function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| } | ||
|
|
||
| __attribute((host, device)) | ||
| void host_device_fun() { | ||
| // warnings expected | ||
| use(__AMDGCN_WAVEFRONT_SIZE__, "host device function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| use(WRAPPED, "host device function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| use(templatify<__AMDGCN_WAVEFRONT_SIZE__>(42), "host device function"); // onhost-warning {{'__hip_device_macro_guard' is deprecated: The __AMDGCN_WAVEFRONT_SIZE macros do not correspond to the device(s) when used in host code and may only be used in device code.}} | ||
| } | ||
|
|
||
| // onhost-note@__clang_hip_device_macro_guards.h:45 0+ {{'__hip_device_macro_guard' has been explicitly marked deprecated here}} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 thought I saw some junk trying to support pre-C++11 HIP, is that a concern here?
Is this macro defined in OpenMP? If so can we do the same thing?
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.
re pre-C++11 HIP: I think we can just drop the
constexprfrom both variants of the guard function; since the guard function is only referenced and never called, the macros would still work as constant expressions.re OpenMP: As far as I can see in experiments, the macros are not defined during OpenMP's host compilation. This is therefore not an issue for OpenMP.
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.
pre-C++11 HIP shouldn't be a concern anymore.