encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-12-20 18:29:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

tint: Flip evaluation order of assignment statements

Browse Source 
Evaluate the LHS before the RHS.

Fixed: tint:1867
Change-Id: Ib63903ed4b1425007197a6da37f3bf54a495d88a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123120
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Ben Clayton
2023-03-08 21:48:45 +00:00
committed by Dawn LUCI CQ
parent 43c5efa7e8
commit da353b4b39
35 changed files with 432 additions and 293 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
src/tint/resolver/uniformity.cc
View File

@@ -45,6 +45,10 @@
// Set to `1` to dump the uniformity graph for each function in graphviz format.
#define TINT_DUMP_UNIFORMITY_GRAPH 0
#if TINT_DUMP_UNIFORMITY_GRAPH
#include <iostream>
#endif
namespace tint::resolver {
namespace {
@@ -123,7 +127,7 @@ struct Node {
const ast::Node* ast = nullptr;
/// The function call argument index, if applicable.
uint32_t arg_index;
uint32_t arg_index = 0xffffffffu;
/// The set of edges from this node to other nodes in the graph.
utils::UniqueVector<Node*, 4> edges;
@@ -547,14 +551,15 @@ class UniformityGraph {
stmt,
[&](const ast::AssignmentStatement* a) {
auto [cf1, v1] = ProcessExpression(cf, a->rhs);
if (a->lhs->Is<ast::PhonyExpression>()) {
return cf1;
} else {
auto [cf2, l2] = ProcessLValueExpression(cf1, a->lhs);
l2->AddEdge(v1);
return cf2;
auto [cf_r, _] = ProcessExpression(cf, a->rhs);
return cf_r;
}
auto [cf_l, v_l, apply] = ProcessLValueExpression(cf, a->lhs);
auto [cf_r, v_r] = ProcessExpression(cf_l, a->rhs);
v_l->AddEdge(v_r);
apply();
return cf_r;
},
[&](const ast::BlockStatement* b) {
@@ -696,17 +701,20 @@ class UniformityGraph {
},
[&](const ast::CompoundAssignmentStatement* c) {
// The compound assignment statement `a += b` is equivalent to `a = a + b`.
// Note: we set load_rule=true when evaluating the LHS the first time, as the
// resolver does not add a load node for it.
auto [cf1, v1] = ProcessExpression(cf, c->lhs, /* load_rule */ true);
auto [cf2, v2] = ProcessExpression(cf1, c->rhs);
// The compound assignment statement `a += b` is equivalent to:
// let p = &a;
// *p = *p + b;
// Note: we set load_rule=true when evaluating the LHS, as the resolver does not add
// a load node for it.
auto [cf1, l1, apply] = ProcessLValueExpression(cf, c->lhs);
auto [cf2, v2] = ProcessExpression(cf1, c->lhs, /* load_rule */ true);
auto [cf3, v3] = ProcessExpression(cf2, c->rhs);
auto* result = CreateNode({"binary_expr_result"});
result->AddEdge(v1);
result->AddEdge(v2);
result->AddEdge(v3);
auto [cf3, l3] = ProcessLValueExpression(cf2, c->lhs);
l3->AddEdge(result);
l1->AddEdge(result);
apply();
return cf3;
},
@@ -965,8 +973,9 @@ class UniformityGraph {
result->AddEdge(v1);
result->AddEdge(cf1);
auto [cf2, l2] = ProcessLValueExpression(cf1, i->lhs);
auto [cf2, l2, apply] = ProcessLValueExpression(cf1, i->lhs);
l2->AddEdge(result);
apply();
return cf2;
},
@@ -1365,48 +1374,62 @@ class UniformityGraph {
return false;
}
/// LValue holds the Nodes returned by ProcessLValueExpression()
struct LValue {
/// The control-flow node for an LValue expression
Node* cf = nullptr;
/// The new value node for an LValue expression
Node* new_val = nullptr;
/// Updates the value node of the LValue expression to be #new_val.
std::function<void()> apply;
};
/// Process an LValue expression.
/// @param cf the input control flow node
/// @param expr the expression to process
/// @returns a pair of (control flow node, variable node)
std::pair<Node*, Node*> ProcessLValueExpression(Node* cf,
const ast::Expression* expr,
bool is_partial_reference = false) {
LValue ProcessLValueExpression(Node* cf,
const ast::Expression* expr,
bool is_partial_reference = false) {
return Switch(
expr,
[&](const ast::IdentifierExpression* i) {
auto* sem = sem_.GetVal(i)->UnwrapLoad()->As<sem::VariableUser>();
if (sem->Variable()->Is<sem::GlobalVariable>()) {
return std::make_pair(cf, current_function_->may_be_non_uniform);
return LValue{cf, current_function_->may_be_non_uniform, [] {}};
} else if (auto* local = sem->Variable()->As<sem::LocalVariable>()) {
// Create a new value node for this variable.
auto* value = CreateNode({NameFor(i), "_lvalue"});
auto* old_value = current_function_->variables.Set(local, value);
auto apply = [=] { current_function_->variables.Set(local, value); };
// If i is part of an expression that is a partial reference to a variable (e.g.
// index or member access), we link back to the variable's previous value. If
// the previous value was non-uniform, a partial assignment will not make it
// uniform.
auto* old_value = current_function_->variables.Get(local);
if (is_partial_reference && old_value) {
value->AddEdge(old_value);
}
return std::make_pair(cf, value);
return LValue{cf, value, apply};
} else {
TINT_ICE(Resolver, diagnostics_)
<< "unknown lvalue identifier expression type: "
<< std::string(sem->Variable()->TypeInfo().name);
return std::pair<Node*, Node*>(nullptr, nullptr);
return LValue{};
}
},
[&](const ast::IndexAccessorExpression* i) {
auto [cf1, l1] =
auto [cf1, l1, apply] =
ProcessLValueExpression(cf, i->object, /*is_partial_reference*/ true);
auto [cf2, v2] = ProcessExpression(cf1, i->index);
l1->AddEdge(v2);
return std::pair<Node*, Node*>(cf2, l1);
return LValue{cf2, l1, apply};
},
[&](const ast::MemberAccessorExpression* m) {
@@ -1419,17 +1442,18 @@ class UniformityGraph {
// that is being written to.
auto* root_ident = sem_.Get(u)->RootIdentifier();
auto* deref = CreateNode({NameFor(root_ident), "_deref"});
auto* old_value = current_function_->variables.Set(root_ident, deref);
if (old_value) {
// If derefercing a partial reference or partial pointer, we link back to
auto apply = [=] { current_function_->variables.Set(root_ident, deref); };
if (auto* old_value = current_function_->variables.Get(root_ident)) {
// If dereferencing a partial reference or partial pointer, we link back to
// the variable's previous value. If the previous value was non-uniform, a
// partial assignment will not make it uniform.
if (is_partial_reference || IsDerefOfPartialPointer(u)) {
deref->AddEdge(old_value);
}
}
return std::pair<Node*, Node*>(cf, deref);
return LValue{cf, deref, apply};
}
return ProcessLValueExpression(cf, u->expr, is_partial_reference);
},
@@ -1437,7 +1461,7 @@ class UniformityGraph {
[&](Default) {
TINT_ICE(Resolver, diagnostics_)
<< "unknown lvalue expression type: " << std::string(expr->TypeInfo().name);
return std::pair<Node*, Node*>(nullptr, nullptr);
return LValue{};
});
}

146
src/tint/resolver/uniformity_test.cc
View File

@@ -8503,5 +8503,151 @@ test:11:7 note: reading from read_write storage buffer 'non_uniform' may result
)");
}
TEST_F(UniformityAnalysisTest, AssignmentEval_LHS_Then_RHS_Pass) {
std::string src = R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : i32;
fn b(p : ptr<function, i32>) -> i32 {
*p = non_uniform;
return 0;
}
fn a(p : ptr<function, i32>) -> i32 {
if (*p == 0) {
workgroupBarrier();
}
return 0;
}
fn foo() {
var i = 0;
var arr : array<i32, 4>;
arr[a(&i)] = arr[b(&i)];
}
)";
RunTest(src, true);
}
TEST_F(UniformityAnalysisTest, AssignmentEval_LHS_Then_RHS_Fail) {
std::string src = R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : i32;
fn a(p : ptr<function, i32>) -> i32 {
*p = non_uniform;
return 0;
}
fn b(p : ptr<function, i32>) -> i32 {
if (*p == 0) {
workgroupBarrier();
}
return 0;
}
fn foo() {
var i = 0;
var arr : array<i32, 4>;
arr[a(&i)] = arr[b(&i)];
}
)";
RunTest(src, false);
EXPECT_EQ(error_,
R"(test:11:5 error: 'workgroupBarrier' must only be called from uniform control flow
workgroupBarrier();
^^^^^^^^^^^^^^^^
test:10:3 note: control flow depends on possibly non-uniform value
if (*p == 0) {
^^
test:10:8 note: parameter 'p' of 'b' may be non-uniform
if (*p == 0) {
^
test:19:22 note: possibly non-uniform value passed via pointer here
arr[a(&i)] = arr[b(&i)];
^
test:19:9 note: contents of pointer may become non-uniform after calling 'a'
arr[a(&i)] = arr[b(&i)];
^
)");
}
TEST_F(UniformityAnalysisTest, CompoundAssignmentEval_LHS_Then_RHS_Pass) {
std::string src = R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : i32;
fn b(p : ptr<function, i32>) -> i32 {
*p = non_uniform;
return 0;
}
fn a(p : ptr<function, i32>) -> i32 {
if (*p == 0) {
workgroupBarrier();
}
return 0;
}
fn foo() {
var i = 0;
var arr : array<i32, 4>;
arr[a(&i)] += arr[b(&i)];
}
)";
RunTest(src, true);
}
TEST_F(UniformityAnalysisTest, CompoundAssignmentEval_LHS_Then_RHS_Fail) {
std::string src = R"(
@group(0) @binding(0) var<storage, read_write> non_uniform : i32;
fn a(p : ptr<function, i32>) -> i32 {
*p = non_uniform;
return 0;
}
fn b(p : ptr<function, i32>) -> i32 {
if (*p == 0) {
workgroupBarrier();
}
return 0;
}
fn foo() {
var i = 0;
var arr : array<i32, 4>;
arr[a(&i)] += arr[b(&i)];
}
)";
RunTest(src, false);
EXPECT_EQ(error_,
R"(test:11:5 error: 'workgroupBarrier' must only be called from uniform control flow
workgroupBarrier();
^^^^^^^^^^^^^^^^
test:10:3 note: control flow depends on possibly non-uniform value
if (*p == 0) {
^^
test:10:8 note: parameter 'p' of 'b' may be non-uniform
if (*p == 0) {
^
test:19:23 note: possibly non-uniform value passed via pointer here
arr[a(&i)] += arr[b(&i)];
^
test:19:9 note: contents of pointer may become non-uniform after calling 'a'
arr[a(&i)] += arr[b(&i)];
^
)");
}
} // namespace
} // namespace tint::resolver

57
src/tint/transform/promote_side_effects_to_decl.cc
View File

@@ -337,9 +337,8 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
});
}
// Starts the recursive processing of a statement's expression(s) to hoist
// side-effects to lets.
void ProcessStatement(const ast::Expression* expr) {
// Starts the recursive processing of a statement's expression(s) to hoist side-effects to lets.
void ProcessExpression(const ast::Expression* expr) {
if (!expr) {
return;
}
@@ -348,31 +347,6 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
ProcessExpression(expr, maybe_hoist);
}
// Special case for processing assignment statement expressions, as we must
// evaluate the rhs before the lhs, and possibly hoist the rhs expression.
void ProcessAssignment(const ast::Expression* lhs, const ast::Expression* rhs) {
// Evaluate rhs before lhs
tint::utils::Vector<const ast::Expression*, 8> maybe_hoist;
if (ProcessExpression(rhs, maybe_hoist)) {
maybe_hoist.Push(rhs);
}
// If the rhs has side-effects, it may affect the lhs, so hoist it right
// away. e.g. "b[c] = a(0);"
if (HasSideEffects(rhs)) {
// Technically, we can always hoist rhs, but don't bother doing so when
// the lhs is just a variable or phony.
if (!lhs->IsAnyOf<ast::IdentifierExpression, ast::PhonyExpression>()) {
Flush(maybe_hoist);
}
}
// If maybe_hoist still has values, it means they are potential side-effect
// receivers. We pass this in while processing the lhs, in which case they
// may get hoisted if the lhs has side-effects. E.g. "b[a(0)] = c;".
ProcessExpression(lhs, maybe_hoist);
}
public:
explicit CollectHoistsState(CloneContext& ctx_in) : StateBase(ctx_in) {}
@@ -386,21 +360,26 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
}
Switch(
stmt, [&](const ast::AssignmentStatement* s) { ProcessAssignment(s->lhs, s->rhs); },
[&](const ast::CallStatement* s) { //
ProcessStatement(s->expr);
stmt, //
[&](const ast::AssignmentStatement* s) {
tint::utils::Vector<const ast::Expression*, 8> maybe_hoist;
ProcessExpression(s->lhs, maybe_hoist);
ProcessExpression(s->rhs, maybe_hoist);
},
[&](const ast::ForLoopStatement* s) { ProcessStatement(s->condition); },
[&](const ast::WhileStatement* s) { ProcessStatement(s->condition); },
[&](const ast::CallStatement* s) { //
ProcessExpression(s->expr);
},
[&](const ast::ForLoopStatement* s) { ProcessExpression(s->condition); },
[&](const ast::WhileStatement* s) { ProcessExpression(s->condition); },
[&](const ast::IfStatement* s) { //
ProcessStatement(s->condition);
ProcessExpression(s->condition);
},
[&](const ast::ReturnStatement* s) { //
ProcessStatement(s->value);
ProcessExpression(s->value);
},
[&](const ast::SwitchStatement* s) { ProcessStatement(s->condition); },
[&](const ast::SwitchStatement* s) { ProcessExpression(s->condition); },
[&](const ast::VariableDeclStatement* s) {
ProcessStatement(s->variable->initializer);
ProcessExpression(s->variable->initializer);
});
}
@@ -563,10 +542,10 @@ class DecomposeSideEffects::DecomposeState : public StateBase {
!sem.GetVal(s->rhs)->HasSideEffects()) {
return nullptr;
}
// rhs before lhs
// lhs before rhs
tint::utils::Vector<const ast::Statement*, 8> stmts;
ctx.Replace(s->rhs, Decompose(s->rhs, &stmts));
ctx.Replace(s->lhs, Decompose(s->lhs, &stmts));
ctx.Replace(s->rhs, Decompose(s->rhs, &stmts));
InsertBefore(stmts, s);
return ctx.CloneWithoutTransform(s);
},

43
src/tint/transform/promote_side_effects_to_decl_test.cc
View File

@@ -2830,9 +2830,8 @@ fn a(i : i32) -> i32 {
fn f() {
var b = array<i32, 10>();
let tint_symbol = a(1);
let tint_symbol_1 = a(0);
b[tint_symbol_1] = tint_symbol;
let tint_symbol = a(0);
b[tint_symbol] = a(1);
}
)";
@@ -2861,10 +2860,9 @@ fn a(i : i32) -> i32 {
fn f() {
var b = array<array<i32, 10>, 10>();
let tint_symbol = a(2);
let tint_symbol_1 = a(0);
let tint_symbol_2 = a(1);
b[tint_symbol_1][tint_symbol_2] = tint_symbol;
let tint_symbol = a(0);
let tint_symbol_1 = a(1);
b[tint_symbol][tint_symbol_1] = a(2);
}
)";
@@ -2893,11 +2891,10 @@ fn a(i : i32) -> i32 {
fn f() {
var b = array<array<array<i32, 10>, 10>, 10>();
let tint_symbol = a(3);
let tint_symbol_1 = a(0);
let tint_symbol_2 = a(1);
let tint_symbol_3 = a(2);
b[tint_symbol_1][tint_symbol_2][tint_symbol_3] = tint_symbol;
let tint_symbol = a(0);
let tint_symbol_1 = a(1);
let tint_symbol_2 = a(2);
b[tint_symbol][tint_symbol_1][tint_symbol_2] = a(3);
}
)";
@@ -2930,11 +2927,9 @@ fn f() {
var b = array<i32, 3>();
var d = array<array<i32, 3>, 3>();
var a_1 = 0;
let tint_symbol = a(0);
let tint_symbol_1 = a_1;
let tint_symbol_2 = d[tint_symbol][tint_symbol_1];
let tint_symbol_3 = a(2);
b[tint_symbol_3] = tint_symbol_2;
let tint_symbol = a(2);
let tint_symbol_1 = a(0);
b[tint_symbol] = d[tint_symbol_1][a_1];
}
)";
@@ -2963,9 +2958,8 @@ fn a(i : i32) -> i32 {
fn f() {
var b = vec3<i32>();
let tint_symbol = a(1);
let tint_symbol_1 = a(0);
b[tint_symbol_1] = tint_symbol;
let tint_symbol = a(0);
b[tint_symbol] = a(1);
}
)";
@@ -2996,9 +2990,8 @@ fn a(i : i32) -> i32 {
fn f() {
var b = vec3<i32>();
var c = 0;
let tint_symbol = c;
let tint_symbol_1 = a(0);
b[tint_symbol_1] = tint_symbol;
let tint_symbol = a(0);
b[tint_symbol] = c;
}
)";
@@ -3029,8 +3022,8 @@ fn a(i : i32) -> i32 {
fn f() {
var b = vec3<i32>();
var c = 0;
let tint_symbol = a(0);
b[c] = tint_symbol;
let tint_symbol = c;
b[tint_symbol] = a(0);
}
)";