[spirv-reader] Even better hoisted variables
Use the fact that in WGSL the scope corresponding to the loop construct
encloses the scope for its associated continue construct.
In our construct data structure, the two are adjacent but not
overlapping.
This improvement means that when a definition is in a loop construct,
but used only in the loop or associated continue construct, then no
hoisting is required.
Bug: tint:3
Change-Id: I8d33b8f76303ab2868306847e846b4c26899e746
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/24420
Reviewed-by: dan sinclair <dsinclair@google.com>
diff --git a/src/reader/spirv/construct.cc b/src/reader/spirv/construct.cc
index ded3d71..2b5c7fc 100644
--- a/src/reader/spirv/construct.cc
+++ b/src/reader/spirv/construct.cc
@@ -24,7 +24,8 @@
uint32_t the_begin_id,
uint32_t the_end_id,
uint32_t the_begin_pos,
- uint32_t the_end_pos)
+ uint32_t the_end_pos,
+ uint32_t the_scope_end_pos)
: parent(the_parent),
enclosing_loop(
// Compute the enclosing loop construct. Doing this in the
@@ -61,7 +62,8 @@
begin_id(the_begin_id),
end_id(the_end_id),
begin_pos(the_begin_pos),
- end_pos(the_end_pos) {}
+ end_pos(the_end_pos),
+ scope_end_pos(the_scope_end_pos) {}
} // namespace spirv
} // namespace reader
diff --git a/src/reader/spirv/construct.h b/src/reader/spirv/construct.h
index 5e6f754..151951a 100644
--- a/src/reader/spirv/construct.h
+++ b/src/reader/spirv/construct.h
@@ -92,19 +92,31 @@
/// @param the_end_id block id of the first block after the construct, or 0
/// @param the_begin_pos block order position of the_begin_id
/// @param the_end_pos block order position of the_end_id or a too-large value
+ /// @param the_scope_end_pos block position of the first block past the end of
+ /// the WGSL scope
Construct(const Construct* the_parent,
int the_depth,
Kind the_kind,
uint32_t the_begin_id,
uint32_t the_end_id,
uint32_t the_begin_pos,
- uint32_t the_end_pos);
+ uint32_t the_end_pos,
+ uint32_t the_scope_end_pos);
/// @param pos a block position
/// @returns true if the given block position is inside this construct.
bool ContainsPos(uint32_t pos) const {
return begin_pos <= pos && pos < end_pos;
}
+ /// Returns true if the given block position is inside the WGSL scope
+ /// corresponding to this construct. A loop construct's WGSL scope encloses
+ /// the associated continue construct. Otherwise the WGSL scope extent is the
+ /// same as the block extent.
+ /// @param pos a block position
+ /// @returns true if the given block position is inside the WGSL scope.
+ bool ScopeContainsPos(uint32_t pos) const {
+ return begin_pos <= pos && pos < scope_end_pos;
+ }
/// The nearest enclosing construct other than itself, or nullptr if
/// this construct represents the entire function.
@@ -136,6 +148,9 @@
/// The position of block |end_id| in the block order, or the number of
/// block order elements if |end_id| is 0.
const uint32_t end_pos = 0;
+ /// The position of the first block after the WGSL scope corresponding to
+ /// this construct.
+ const uint32_t scope_end_pos = 0;
};
using ConstructList = std::vector<std::unique_ptr<Construct>>;
@@ -183,6 +198,10 @@
o << " parent:" << ToStringBrief(c.parent);
+ if (c.scope_end_pos != c.end_pos) {
+ o << " scope:[" << c.begin_pos << "," << c.scope_end_pos << ")";
+ }
+
if (c.enclosing_loop) {
o << " in-l:" << ToStringBrief(c.enclosing_loop);
}
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index df4dc52..b5bcb9d 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -903,16 +903,18 @@
const auto end_pos =
end_id == 0 ? uint32_t(block_order_.size()) : GetBlockInfo(end_id)->pos;
const auto* parent = enclosing.empty() ? nullptr : enclosing.back();
+ auto scope_end_pos = end_pos;
// A loop construct is added right after its associated continue construct.
// In that case, adjust the parent up.
if (k == Construct::kLoop) {
assert(parent);
assert(parent->kind == Construct::kContinue);
+ scope_end_pos = parent->end_pos;
parent = parent->parent;
}
- constructs_.push_back(
- std::make_unique<Construct>(parent, static_cast<int>(depth), k,
- begin_id, end_id, begin_pos, end_pos));
+ constructs_.push_back(std::make_unique<Construct>(
+ parent, static_cast<int>(depth), k, begin_id, end_id, begin_pos,
+ end_pos, scope_end_pos));
Construct* result = constructs_.back().get();
enclosing.push_back(result);
return result;
@@ -3137,7 +3139,8 @@
GetBlockInfo(block_order_[first_pos])->construct;
assert(enclosing_construct != nullptr);
// Constructs are strictly nesting, so follow parent pointers
- while (enclosing_construct && !enclosing_construct->ContainsPos(last_pos)) {
+ while (enclosing_construct &&
+ !enclosing_construct->ScopeContainsPos(last_pos)) {
// The scope of a continue construct is enclosed in its associated loop
// construct, but they are siblings in our construct tree.
const auto* sibling_loop = SiblingLoopConstruct(enclosing_construct);
diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc
index 188b3b9..7e38f97 100644
--- a/src/reader/spirv/function_cfg_test.cc
+++ b/src/reader/spirv/function_cfg_test.cc
@@ -3100,7 +3100,7 @@
EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
Construct{ Function [0,6) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ Continue [3,5) begin_id:40 end_id:99 depth:1 parent:Function@10 in-c:Continue@40 }
- Construct{ Loop [1,3) begin_id:20 end_id:40 depth:1 parent:Function@10 in-l:Loop@20 }
+ Construct{ Loop [1,3) begin_id:20 end_id:40 depth:1 parent:Function@10 scope:[1,5) in-l:Loop@20 }
})")) << constructs;
// The block records the nearest enclosing construct.
EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get());
@@ -3244,7 +3244,7 @@
Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ IfSelection [0,2) begin_id:10 end_id:50 depth:1 parent:Function@10 }
Construct{ Continue [3,4) begin_id:60 end_id:99 depth:1 parent:Function@10 in-c:Continue@60 }
- Construct{ Loop [2,3) begin_id:50 end_id:60 depth:1 parent:Function@10 in-l:Loop@50 }
+ Construct{ Loop [2,3) begin_id:50 end_id:60 depth:1 parent:Function@10 scope:[2,4) in-l:Loop@50 }
})")) << constructs;
// The block records the nearest enclosing construct.
EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -3466,7 +3466,7 @@
EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
Construct{ Function [0,8) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ Continue [4,6) begin_id:50 end_id:89 depth:1 parent:Function@10 in-c:Continue@50 }
- Construct{ Loop [1,4) begin_id:20 end_id:50 depth:1 parent:Function@10 in-l:Loop@20 }
+ Construct{ Loop [1,4) begin_id:20 end_id:50 depth:1 parent:Function@10 scope:[1,6) in-l:Loop@20 }
Construct{ Continue [2,3) begin_id:30 end_id:40 depth:2 parent:Loop@20 in-l:Loop@20 in-c:Continue@30 }
})")) << constructs;
// The block records the nearest enclosing construct.
@@ -3521,7 +3521,7 @@
EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
Construct{ Function [0,7) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ Continue [5,6) begin_id:80 end_id:99 depth:1 parent:Function@10 in-c:Continue@80 }
- Construct{ Loop [1,5) begin_id:20 end_id:80 depth:1 parent:Function@10 in-l:Loop@20 }
+ Construct{ Loop [1,5) begin_id:20 end_id:80 depth:1 parent:Function@10 scope:[1,6) in-l:Loop@20 }
Construct{ IfSelection [2,4) begin_id:30 end_id:49 depth:2 parent:Loop@20 in-l:Loop@20 }
})")) << constructs;
// The block records the nearest enclosing construct.
@@ -3572,7 +3572,7 @@
EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
Construct{ Function [0,6) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ Continue [2,5) begin_id:30 end_id:99 depth:1 parent:Function@10 in-c:Continue@30 }
- Construct{ Loop [1,2) begin_id:20 end_id:30 depth:1 parent:Function@10 in-l:Loop@20 }
+ Construct{ Loop [1,2) begin_id:20 end_id:30 depth:1 parent:Function@10 scope:[1,5) in-l:Loop@20 }
Construct{ IfSelection [2,4) begin_id:30 end_id:49 depth:2 parent:Continue@30 in-c:Continue@30 }
})")) << constructs;
// The block records the nearest enclosing construct.
@@ -3666,7 +3666,7 @@
Construct{ Function [0,7) begin_id:10 end_id:0 depth:0 parent:null }
Construct{ IfSelection [0,6) begin_id:10 end_id:99 depth:1 parent:Function@10 }
Construct{ Continue [3,5) begin_id:40 end_id:89 depth:2 parent:IfSelection@10 in-c:Continue@40 }
- Construct{ Loop [1,3) begin_id:20 end_id:40 depth:2 parent:IfSelection@10 in-l:Loop@20 }
+ Construct{ Loop [1,3) begin_id:20 end_id:40 depth:2 parent:IfSelection@10 scope:[1,5) in-l:Loop@20 }
})")) << constructs;
// The block records the nearest enclosing construct.
EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -13538,7 +13538,7 @@
EXPECT_EQ(c->kind, Construct::kContinue);
EXPECT_THAT(ToString(fe.SiblingLoopConstruct(c)),
Eq("Construct{ Loop [1,2) begin_id:20 end_id:30 depth:1 "
- "parent:Function@10 in-l:Loop@20 }"));
+ "parent:Function@10 scope:[1,3) in-l:Loop@20 }"));
}
} // namespace
diff --git a/src/reader/spirv/function_var_test.cc b/src/reader/spirv/function_var_test.cc
index c591175..c160a23 100644
--- a/src/reader/spirv/function_var_test.cc
+++ b/src/reader/spirv/function_var_test.cc
@@ -1125,13 +1125,6 @@
EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{
VariableDeclStatement{
Variable{
- x_2
- function
- __u32
- }
- }
- VariableDeclStatement{
- Variable{
x_2_phi
function
__u32
@@ -1181,9 +1174,15 @@
}
}
Loop{
- Assignment{
- Identifier{x_2}
- Identifier{x_2_phi}
+ VariableDeclStatement{
+ Variable{
+ x_2
+ none
+ __u32
+ {
+ Identifier{x_2_phi}
+ }
+ }
}
VariableDeclStatement{
Variable{
@@ -1291,20 +1290,6 @@
Loop{
VariableDeclStatement{
Variable{
- x_4
- function
- __u32
- }
- }
- VariableDeclStatement{
- Variable{
- x_6
- function
- __u32
- }
- }
- VariableDeclStatement{
- Variable{
x_2_phi
function
__u32
@@ -1346,20 +1331,32 @@
}
}
}
- Assignment{
- Identifier{x_4}
- Binary{
- Identifier{x_2}
- add
- ScalarConstructor{1}
+ VariableDeclStatement{
+ Variable{
+ x_4
+ none
+ __u32
+ {
+ Binary{
+ Identifier{x_2}
+ add
+ ScalarConstructor{1}
+ }
+ }
}
}
- Assignment{
- Identifier{x_6}
- Binary{
- Identifier{x_4}
- add
- ScalarConstructor{1}
+ VariableDeclStatement{
+ Variable{
+ x_6
+ none
+ __u32
+ {
+ Binary{
+ Identifier{x_4}
+ add
+ ScalarConstructor{1}
+ }
+ }
}
}
If{
