Skip to content

[AMDGPU] Report error in clang if wave32 is requested where unsupported #97633

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

Merged
merged 5 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions clang/lib/Basic/Targets/AMDGPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,15 @@ bool AMDGPUTargetInfo::initFeatureMap(
return false;

// TODO: Should move this logic into TargetParser
std::string ErrorMsg;
if (!insertWaveSizeFeature(CPU, getTriple(), Features, ErrorMsg)) {
Diags.Report(diag::err_invalid_feature_combination) << ErrorMsg;
auto HasError = insertWaveSizeFeature(CPU, getTriple(), Features);
switch (HasError.first) {
default:
break;
case llvm::AMDGPU::INVALID_FEATURE_COMBINATION:
Diags.Report(diag::err_invalid_feature_combination) << HasError.second;
return false;
case llvm::AMDGPU::UNSUPPORTED_TARGET_FEATURE:
Diags.Report(diag::err_opt_not_valid_on_target) << HasError.second;
return false;
}

Expand Down
2 changes: 2 additions & 0 deletions clang/test/CodeGenOpenCL/amdgpu-features-illegal.cl
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// RUN: not %clang_cc1 -triple amdgcn -target-feature +wavefrontsize32 -target-feature +wavefrontsize64 -o /dev/null %s 2>&1 | FileCheck %s
// RUN: not %clang_cc1 -triple amdgcn -target-cpu gfx1103 -target-feature +wavefrontsize32 -target-feature +wavefrontsize64 -o /dev/null %s 2>&1 | FileCheck %s
// RUN: not %clang_cc1 -triple amdgcn -target-cpu gfx900 -target-feature +wavefrontsize32 -o /dev/null %s 2>&1 | FileCheck %s --check-prefix=GFX9

// CHECK: error: invalid feature combination: 'wavefrontsize32' and 'wavefrontsize64' are mutually exclusive
// GFX9: error: option 'wavefrontsize32' cannot be specified on this target

kernel void test() {}
3 changes: 1 addition & 2 deletions clang/test/SemaOpenCL/builtins-amdgcn-error-wave32.cl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ void test_ballot_wave32(global uint* out, int a, int b) {
*out = __builtin_amdgcn_ballot_w32(a == b); // expected-error {{'__builtin_amdgcn_ballot_w32' needs target feature wavefrontsize32}}
}

// FIXME: Should error for subtargets that don't support wave32
__attribute__((target("wavefrontsize32")))
__attribute__((target("wavefrontsize32"))) // gfx9-error@*:* {{option 'wavefrontsize32' cannot be specified on this target}}
void test_ballot_wave32_target_attr(global uint* out, int a, int b) {
*out = __builtin_amdgcn_ballot_w32(a == b);
}
11 changes: 9 additions & 2 deletions llvm/include/llvm/TargetParser/TargetParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ enum ArchFeatureKind : uint32_t {
FEATURE_WGP = 1 << 9,
};

enum FeatureError : uint32_t {
NO_ERROR = 0,
INVALID_FEATURE_COMBINATION,
UNSUPPORTED_TARGET_FEATURE
};

StringRef getArchFamilyNameAMDGCN(GPUKind AK);

StringRef getArchNameAMDGCN(GPUKind AK);
Expand All @@ -177,8 +183,9 @@ void fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
StringMap<bool> &Features);

/// Inserts wave size feature for given GPU into features map
bool insertWaveSizeFeature(StringRef GPU, const Triple &T,
StringMap<bool> &Features, std::string &ErrorMsg);
std::pair<FeatureError, StringRef>
insertWaveSizeFeature(StringRef GPU, const Triple &T,
StringMap<bool> &Features);

} // namespace AMDGPU
} // namespace llvm
Expand Down
19 changes: 10 additions & 9 deletions llvm/lib/TargetParser/TargetParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,18 +616,19 @@ static bool isWave32Capable(StringRef GPU, const Triple &T) {
return IsWave32Capable;
}

bool AMDGPU::insertWaveSizeFeature(StringRef GPU, const Triple &T,
StringMap<bool> &Features,
std::string &ErrorMsg) {
std::pair<FeatureError, StringRef>
AMDGPU::insertWaveSizeFeature(StringRef GPU, const Triple &T,
StringMap<bool> &Features) {
bool IsWave32Capable = isWave32Capable(GPU, T);
const bool IsNullGPU = GPU.empty();
// FIXME: Not diagnosing wavefrontsize32 on wave64 only targets.
const bool HaveWave32 =
(IsWave32Capable || IsNullGPU) && Features.count("wavefrontsize32");
const bool HaveWave32 = Features.count("wavefrontsize32");
const bool HaveWave64 = Features.count("wavefrontsize64");
if (HaveWave32 && HaveWave64) {
ErrorMsg = "'wavefrontsize32' and 'wavefrontsize64' are mutually exclusive";
return false;
return {AMDGPU::INVALID_FEATURE_COMBINATION,
"'wavefrontsize32' and 'wavefrontsize64' are mutually exclusive"};
}
if (HaveWave32 && !IsNullGPU && !IsWave32Capable) {
return {AMDGPU::UNSUPPORTED_TARGET_FEATURE, "wavefrontsize32"};
}
// Don't assume any wavesize with an unknown subtarget.
if (!IsNullGPU) {
Expand All @@ -638,5 +639,5 @@ bool AMDGPU::insertWaveSizeFeature(StringRef GPU, const Triple &T,
Features.insert(std::make_pair(DefaultWaveSizeFeature, true));
}
}
return true;
return {NO_ERROR, StringRef()};
}
Loading