Skip to content

Commit

Permalink
[tint] Require uniformity for subgroupShuffle* delta values
Browse files Browse the repository at this point in the history
Allow the severity of this diagnostic to be overridden with the
`subgroup_uniformity` diagnostic rule.

Bug: 369426560
Change-Id: Id222d95cd2f0b2b6212dce50d4da15bffb518de7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214896
Reviewed-by: David Neto <[email protected]>
Commit-Queue: James Price <[email protected]>
  • Loading branch information
jrprice authored and Dawn LUCI CQ committed Nov 15, 2024
1 parent b7ecabd commit dcca7d2
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 2 deletions.
13 changes: 13 additions & 0 deletions src/tint/lang/wgsl/resolver/uniformity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,19 @@ class UniformityGraph {
if (builtin && builtin->Fn() == wgsl::BuiltinFn::kWorkgroupUniformLoad) {
// The workgroupUniformLoad builtin requires its parameter to be uniform.
current_function_->RequiredToBeUniform(default_severity)->AddEdge(args[i]);
} else if (builtin &&
(builtin->Fn() == wgsl::BuiltinFn::kSubgroupShuffleDown ||
builtin->Fn() == wgsl::BuiltinFn::kSubgroupShuffleUp ||
builtin->Fn() == wgsl::BuiltinFn::kSubgroupShuffleXor) &&
i == 1) {
// The subgroupShuffle{Down,Up,Xor} builtins require their `delta` parameters to
// be uniform.
// Get the severity of subgroup uniformity violations in this context.
auto severity = sem_.DiagnosticSeverity(
call->args[i], wgsl::CoreDiagnosticRule::kSubgroupUniformity);
if (severity != wgsl::DiagnosticSeverity::kOff) {
current_function_->RequiredToBeUniform(severity)->AddEdge(args[i]);
}
} else {
// All other builtin function parameters are RequiredToBeUniformForReturnValue,
// as are parameters for value constructors and value conversions.
Expand Down
121 changes: 119 additions & 2 deletions src/tint/lang/wgsl/resolver/uniformity_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9092,6 +9092,75 @@ test:12:17 note: builtin 'idx' of 'main' may be non-uniform
)");
}

TEST_F(UniformityAnalysisTest, SubgroupShuffleDown_Delta) {
std::string src = R"(
enable subgroups;
var<private> delta: u32;
fn foo() {
_ = subgroupShuffleDown(1.0, delta);
}
)";

RunTest(src, false);
EXPECT_EQ(error_,
R"(test:7:32 error: 'subgroupShuffleDown' requires argument 1 to be uniform
_ = subgroupShuffleDown(1.0, delta);
^^^^^
test:7:32 note: reading from module-scope private variable 'delta' may result in a non-uniform value
_ = subgroupShuffleDown(1.0, delta);
^^^^^
)");
}

TEST_F(UniformityAnalysisTest, SubgroupShuffleUp_Delta) {
std::string src = R"(
enable subgroups;
var<private> delta: u32;
fn foo() {
_ = subgroupShuffleUp(1.0, delta);
}
)";

RunTest(src, false);
EXPECT_EQ(error_,
R"(test:7:30 error: 'subgroupShuffleUp' requires argument 1 to be uniform
_ = subgroupShuffleUp(1.0, delta);
^^^^^
test:7:30 note: reading from module-scope private variable 'delta' may result in a non-uniform value
_ = subgroupShuffleUp(1.0, delta);
^^^^^
)");
}

TEST_F(UniformityAnalysisTest, SubgroupShuffleXor_Delta) {
std::string src = R"(
enable subgroups;
var<private> delta: u32;
fn foo() {
_ = subgroupShuffleXor(1.0, delta);
}
)";

RunTest(src, false);
EXPECT_EQ(error_,
R"(test:7:31 error: 'subgroupShuffleXor' requires argument 1 to be uniform
_ = subgroupShuffleXor(1.0, delta);
^^^^^
test:7:31 note: reading from module-scope private variable 'delta' may result in a non-uniform value
_ = subgroupShuffleXor(1.0, delta);
^^^^^
)");
}

TEST_F(UniformityAnalysisTest, WorkgroupAtomics) {
std::string src = R"(
var<workgroup> a : atomic<i32>;
Expand Down Expand Up @@ -9288,7 +9357,7 @@ fn foo() {
}
}

TEST_P(UniformityAnalysisDiagnosticFilterTest, Directive_SubgroupUniformity) {
TEST_P(UniformityAnalysisDiagnosticFilterTest, Directive_SubgroupUniformity_Callsite) {
auto& param = GetParam();
StringStream ss;
ss << "enable subgroups;\n"
Expand All @@ -9313,6 +9382,29 @@ fn foo() {
}
}

TEST_P(UniformityAnalysisDiagnosticFilterTest, Directive_SubgroupUniformity_ShuffleDelta) {
auto& param = GetParam();
StringStream ss;
ss << "enable subgroups;\n"
<< "diagnostic(" << param << ", subgroup_uniformity);" << R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : u32;
fn foo() {
_ = subgroupShuffleUp(1.0, non_uniform);
}
)";

RunTest(ss.str(), param != wgsl::DiagnosticSeverity::kError);

if (param == wgsl::DiagnosticSeverity::kOff) {
EXPECT_TRUE(error_.empty());
} else {
StringStream err;
err << ToStr(param) << ": 'subgroupShuffleUp' requires argument 1 to be uniform";
EXPECT_THAT(error_, ::testing::HasSubstr(err.str()));
}
}

TEST_P(UniformityAnalysisDiagnosticFilterTest, AttributeOnFunction_DerivativeUniformity) {
auto& param = GetParam();
StringStream ss;
Expand All @@ -9339,7 +9431,7 @@ TEST_P(UniformityAnalysisDiagnosticFilterTest, AttributeOnFunction_DerivativeUni
}
}

TEST_P(UniformityAnalysisDiagnosticFilterTest, AttributeOnFunction_SubgroupUniformity) {
TEST_P(UniformityAnalysisDiagnosticFilterTest, AttributeOnFunction_SubgroupUniformity_Callsite) {
auto& param = GetParam();
StringStream ss;
ss << R"(
Expand All @@ -9364,6 +9456,31 @@ enable subgroups;
}
}

TEST_P(UniformityAnalysisDiagnosticFilterTest,
AttributeOnFunction_SubgroupUniformity_ShuffleDelta) {
auto& param = GetParam();
StringStream ss;
ss << "enable subgroups;\n"
<< "diagnostic(" << param << ", subgroup_uniformity);" << R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : u32;
)" << "@diagnostic("
<< param << ", subgroup_uniformity)" <<
R"(fn foo() {
_ = subgroupShuffleUp(1.0, non_uniform);
}
)";

RunTest(ss.str(), param != wgsl::DiagnosticSeverity::kError);

if (param == wgsl::DiagnosticSeverity::kOff) {
EXPECT_TRUE(error_.empty());
} else {
StringStream err;
err << ToStr(param) << ": 'subgroupShuffleUp' requires argument 1 to be uniform";
EXPECT_THAT(error_, ::testing::HasSubstr(err.str()));
}
}

// Test the various places that the attributes can be used against just one diagnostic rule, to
// avoid over-parameterizing the tests.

Expand Down

0 comments on commit dcca7d2

Please sign in to comment.