[ir] Fix ValueToLet for store-before-load
Function calls that may store were sometimes being inlined over load
instructions. Track the fact that there are accesses pending
resolution that contain stores to fix this.
Change-Id: Idba741e83f0502a897e71ee8966e01452ca5d3f0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/191660
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/ir/transform/value_to_let.cc b/src/tint/lang/core/ir/transform/value_to_let.cc
index ac51672..ec25a39 100644
--- a/src/tint/lang/core/ir/transform/value_to_let.cc
+++ b/src/tint/lang/core/ir/transform/value_to_let.cc
@@ -141,6 +141,7 @@
if (accesses.Contains(Access::kStore)) { // Note: Also handles load + store
put_pending_in_lets();
+ pending_access = Access::kStore;
maybe_put_in_let(inst);
} else if (accesses.Contains(Access::kLoad)) {
if (pending_access != Access::kLoad) {
diff --git a/src/tint/lang/core/ir/transform/value_to_let_test.cc b/src/tint/lang/core/ir/transform/value_to_let_test.cc
index 46e1c12..d88039e 100644
--- a/src/tint/lang/core/ir/transform/value_to_let_test.cc
+++ b/src/tint/lang/core/ir/transform/value_to_let_test.cc
@@ -29,6 +29,7 @@
#include <utility>
+#include "gtest/gtest.h"
#include "src/tint/lang/core/ir/transform/helper_test.h"
namespace tint::core::ir::transform {
@@ -45,7 +46,7 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_Blah) {
@@ -59,13 +60,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_Unsequenced) {
@@ -88,13 +89,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_SequencedValueUsedWithNonSequenced) {
auto* i = b.Var<private_, i32>("i");
@@ -139,13 +140,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_Inlinable_NestedCalls) {
@@ -192,13 +193,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_LetUsedTwice) {
@@ -245,13 +246,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NoModify_VarUsedTwice) {
@@ -287,13 +288,13 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = src;
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, VarLoadUsedTwice) {
@@ -314,7 +315,7 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
%F = func():i32 {
@@ -330,7 +331,7 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, VarLoad_ThenStore_ThenUse) {
@@ -352,7 +353,7 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
%F = func():i32 {
@@ -368,7 +369,141 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
+}
+
+TEST_F(IR_ValueToLetTest, Call_ThenLoad_ThenUseCallBeforeLoad) {
+ auto* v = b.Var<private_, i32>("v");
+ mod.root_block->Append(v);
+
+ auto* foo = b.Function("foo", ty.i32());
+ b.Append(foo->Block(), [&] {
+ b.Store(v, 42_i);
+ b.Return(foo, 1_i);
+ });
+
+ auto* fn = b.Function("F", ty.i32());
+ b.Append(fn->Block(), [&] {
+ auto* c = b.Call<i32>(foo);
+ auto* l = b.Name("l", b.Load(v));
+ auto* add = b.Name("add", b.Add<i32>(c, l));
+ b.Return(fn, add);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %v:ptr<private, i32, read_write> = var
+}
+
+%foo = func():i32 {
+ $B2: {
+ store %v, 42i
+ ret 1i
+ }
+}
+%F = func():i32 {
+ $B3: {
+ %4:i32 = call %foo
+ %l:i32 = load %v
+ %add:i32 = add %4, %l
+ ret %add
+ }
+}
+)";
+ EXPECT_EQ(str(), src);
+
+ auto* expect = R"(
+$B1: { # root
+ %v:ptr<private, i32, read_write> = var
+}
+
+%foo = func():i32 {
+ $B2: {
+ store %v, 42i
+ ret 1i
+ }
+}
+%F = func():i32 {
+ $B3: {
+ %4:i32 = call %foo
+ %5:i32 = let %4
+ %l:i32 = load %v
+ %add:i32 = add %5, %l
+ ret %add
+ }
+}
+)";
+
+ Run(ValueToLet);
+
+ EXPECT_EQ(str(), expect);
+}
+
+TEST_F(IR_ValueToLetTest, Call_ThenLoad_ThenUseLoadBeforeCall) {
+ auto* v = b.Var<private_, i32>("v");
+ mod.root_block->Append(v);
+
+ auto* foo = b.Function("foo", ty.i32());
+ b.Append(foo->Block(), [&] {
+ b.Store(v, 42_i);
+ b.Return(foo, 1_i);
+ });
+
+ auto* fn = b.Function("F", ty.i32());
+ b.Append(fn->Block(), [&] {
+ auto* c = b.Call<i32>(foo);
+ auto* l = b.Name("l", b.Load(v));
+ auto* add = b.Name("add", b.Add<i32>(l, c));
+ b.Return(fn, add);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %v:ptr<private, i32, read_write> = var
+}
+
+%foo = func():i32 {
+ $B2: {
+ store %v, 42i
+ ret 1i
+ }
+}
+%F = func():i32 {
+ $B3: {
+ %4:i32 = call %foo
+ %l:i32 = load %v
+ %add:i32 = add %l, %4
+ ret %add
+ }
+}
+)";
+ EXPECT_EQ(str(), src);
+
+ auto* expect = R"(
+$B1: { # root
+ %v:ptr<private, i32, read_write> = var
+}
+
+%foo = func():i32 {
+ $B2: {
+ store %v, 42i
+ ret 1i
+ }
+}
+%F = func():i32 {
+ $B3: {
+ %4:i32 = call %foo
+ %5:i32 = let %4
+ %l:i32 = load %v
+ %add:i32 = add %l, %5
+ ret %add
+ }
+}
+)";
+
+ Run(ValueToLet);
+
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, TwoCalls_ThenUseReturnValues) {
@@ -415,7 +550,7 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
$B1: { # root
@@ -444,11 +579,11 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
Run(ValueToLet); // running a second time should be no-op
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, SequencedUsedInDifferentBlock) {
@@ -500,7 +635,7 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
$B1: { # root
@@ -532,11 +667,11 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
Run(ValueToLet); // running a second time should be no-op
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NameMe1) {
@@ -560,7 +695,7 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
%F = func():i32 {
@@ -577,11 +712,11 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
Run(ValueToLet); // running a second time should be no-op
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
TEST_F(IR_ValueToLetTest, NameMe2) {
@@ -609,14 +744,15 @@
}
}
)";
- EXPECT_EQ(src, str());
+ EXPECT_EQ(str(), src);
auto* expect = R"(
%F = func():void {
$B1: {
%i:i32 = max 1i, 2i
%v:ptr<function, i32, read_write> = var, %i
- %x:i32 = max 3i, 4i
+ %4:i32 = max 3i, 4i
+ %x:i32 = let %4
%y:i32 = load %v
%z:i32 = add %y, %x
store %v, %z
@@ -627,11 +763,11 @@
Run(ValueToLet);
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
Run(ValueToLet); // running a second time should be no-op
- EXPECT_EQ(expect, str());
+ EXPECT_EQ(str(), expect);
}
} // namespace
} // namespace tint::core::ir::transform
diff --git a/test/tint/bug/tint/1118.wgsl.expected.ir.msl b/test/tint/bug/tint/1118.wgsl.expected.ir.msl
index 82729f0a..984cfe6 100644
--- a/test/tint/bug/tint/1118.wgsl.expected.ir.msl
+++ b/test/tint/bug/tint/1118.wgsl.expected.ir.msl
@@ -114,10 +114,11 @@
(*tint_module_vars.fClipDistance3) = fClipDistance3_param;
(*tint_module_vars.fClipDistance4) = fClipDistance4_param;
main_1(tint_module_vars);
+ main_out const v_3 = main_out{.glFragColor_1=(*tint_module_vars.glFragColor)};
if (!((*tint_module_vars.continue_execution))) {
discard_fragment();
}
- return main_out{.glFragColor_1=(*tint_module_vars.glFragColor)};
+ return v_3;
}
fragment tint_symbol_outputs tint_symbol(tint_symbol_inputs inputs [[stage_in]], const constant Scene* x_29 [[buffer(0)]], const constant Material* x_49 [[buffer(1)]], const constant Mesh* x_137 [[buffer(2)]]) {
thread float fClipDistance3 = 0.0f;
diff --git a/test/tint/expressions/user_call/multi_param_return.wgsl.expected.ir.msl b/test/tint/expressions/user_call/multi_param_return.wgsl.expected.ir.msl
index d5a8840..bf9fdcb 100644
--- a/test/tint/expressions/user_call/multi_param_return.wgsl.expected.ir.msl
+++ b/test/tint/expressions/user_call/multi_param_return.wgsl.expected.ir.msl
@@ -8,5 +8,6 @@
}
void b() {
int b_1 = c(2, 3, 4);
- b_1 = (b_1 + c(3, 4, 5));
+ int const v = c(3, 4, 5);
+ b_1 = (b_1 + v);
}
diff --git a/test/tint/expressions/user_call/no_params_return.wgsl.expected.ir.msl b/test/tint/expressions/user_call/no_params_return.wgsl.expected.ir.msl
index bb96547..32aa59d 100644
--- a/test/tint/expressions/user_call/no_params_return.wgsl.expected.ir.msl
+++ b/test/tint/expressions/user_call/no_params_return.wgsl.expected.ir.msl
@@ -8,5 +8,6 @@
}
void b() {
int b_1 = c();
- b_1 = (b_1 + c());
+ int const v = c();
+ b_1 = (b_1 + v);
}
diff --git a/test/tint/expressions/user_call/one_param_return.wgsl.expected.ir.msl b/test/tint/expressions/user_call/one_param_return.wgsl.expected.ir.msl
index b98d67f..407a01b 100644
--- a/test/tint/expressions/user_call/one_param_return.wgsl.expected.ir.msl
+++ b/test/tint/expressions/user_call/one_param_return.wgsl.expected.ir.msl
@@ -8,5 +8,6 @@
}
void b() {
int b_1 = c(2);
- b_1 = (b_1 + c(3));
+ int const v = c(3);
+ b_1 = (b_1 + v);
}
