Fix nits from review
From review https://dawn-review.googlesource.com/c/tint/+/20080
- single_block_loop -> is_single_block_loop
- better message for internal error for missing block when registering
merges
Bug: tint:3
Change-Id: Ief8ac5ce8ad7ffe93c28e0e7e2a793d50ce2de6c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/20345
Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 31fc1c7..4883ebb 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -390,7 +390,8 @@
const auto block_id = block.id();
auto* block_info = GetBlockInfo(block_id);
if (!block_info) {
- return Fail() << "internal error: assumed blocks were registered";
+ return Fail() << "internal error: block " << block_id << " missing; blocks should already "
+ "have been registered";
}
if (const auto* inst = block.GetMergeInst()) {
@@ -464,18 +465,18 @@
}
// Check single-block loop cases.
- bool single_block_loop = false;
+ bool is_single_block_loop = false;
block_info->basic_block->ForEachSuccessorLabel(
- [&single_block_loop, block_id](const uint32_t succ) {
+ [&is_single_block_loop, block_id](const uint32_t succ) {
if (block_id == succ)
- single_block_loop = true;
+ is_single_block_loop = true;
});
- block_info->single_block_loop = single_block_loop;
+ block_info->is_single_block_loop = is_single_block_loop;
const auto ct = block_info->continue_for_header;
- if (single_block_loop && ct != block_id) {
+ if (is_single_block_loop && ct != block_id) {
return Fail() << "Block " << block_id
<< " branches to itself but is not its own continue target";
- } else if (!single_block_loop && ct == block_id) {
+ } else if (!is_single_block_loop && ct == block_id) {
return Fail() << "Loop header block " << block_id
<< " declares itself as its own continue target, but "
"does not branch to itself";
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index 73d8cd8..46f15d7 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -64,7 +64,7 @@
uint32_t header_for_continue = 0;
/// Is this block a single-block loop: A loop header that declares itself
/// as its own continue target, and has branch to itself.
- bool single_block_loop = false;
+ bool is_single_block_loop = false;
};
/// A FunctionEmitter emits a SPIR-V function onto a Tint AST module.
@@ -113,7 +113,7 @@
/// @returns true if terminators are sane
bool TerminatorsAreSane();
- /// Populates merge-header cross-links and the |single_block_loop| member
+ /// Populates merge-header cross-links and the |is_single_block_loop| member
/// of BlockInfo. Also verifies that merge instructions go to blocks in
/// the same function. Assumes basic blocks have been registered, and
/// terminators are sane.
diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc
index 397d1d3..d86182d 100644
--- a/src/reader/spirv/function_cfg_test.cc
+++ b/src/reader/spirv/function_cfg_test.cc
@@ -309,7 +309,7 @@
EXPECT_EQ(bi->continue_for_header, 0u);
EXPECT_EQ(bi->header_for_merge, 0u);
EXPECT_EQ(bi->header_for_continue, 0u);
- EXPECT_FALSE(bi->single_block_loop);
+ EXPECT_FALSE(bi->is_single_block_loop);
}
TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) {
@@ -340,7 +340,7 @@
EXPECT_EQ(bi10->continue_for_header, 0u);
EXPECT_EQ(bi10->header_for_merge, 0u);
EXPECT_EQ(bi10->header_for_continue, 0u);
- EXPECT_FALSE(bi10->single_block_loop);
+ EXPECT_FALSE(bi10->is_single_block_loop);
// Middle block is neither header nor merge
const auto* bi20 = fe.GetBlockInfo(20);
@@ -349,7 +349,7 @@
EXPECT_EQ(bi20->continue_for_header, 0u);
EXPECT_EQ(bi20->header_for_merge, 0u);
EXPECT_EQ(bi20->header_for_continue, 0u);
- EXPECT_FALSE(bi20->single_block_loop);
+ EXPECT_FALSE(bi20->is_single_block_loop);
// Merge block points to the header
const auto* bi99 = fe.GetBlockInfo(99);
@@ -358,7 +358,7 @@
EXPECT_EQ(bi99->continue_for_header, 0u);
EXPECT_EQ(bi99->header_for_merge, 10u);
EXPECT_EQ(bi99->header_for_continue, 0u);
- EXPECT_FALSE(bi99->single_block_loop);
+ EXPECT_FALSE(bi99->is_single_block_loop);
}
TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) {
@@ -389,7 +389,7 @@
EXPECT_EQ(bi10->continue_for_header, 0u);
EXPECT_EQ(bi10->header_for_merge, 0u);
EXPECT_EQ(bi10->header_for_continue, 0u);
- EXPECT_FALSE(bi10->single_block_loop);
+ EXPECT_FALSE(bi10->is_single_block_loop);
// Middle block is neither header nor merge
const auto* bi20 = fe.GetBlockInfo(20);
@@ -398,7 +398,7 @@
EXPECT_EQ(bi20->continue_for_header, 0u);
EXPECT_EQ(bi20->header_for_merge, 0u);
EXPECT_EQ(bi20->header_for_continue, 0u);
- EXPECT_FALSE(bi20->single_block_loop);
+ EXPECT_FALSE(bi20->is_single_block_loop);
// Merge block points to the header
const auto* bi99 = fe.GetBlockInfo(99);
@@ -407,7 +407,7 @@
EXPECT_EQ(bi99->continue_for_header, 0u);
EXPECT_EQ(bi99->header_for_merge, 10u);
EXPECT_EQ(bi99->header_for_continue, 0u);
- EXPECT_FALSE(bi99->single_block_loop);
+ EXPECT_FALSE(bi99->is_single_block_loop);
}
TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) {
@@ -438,7 +438,7 @@
EXPECT_EQ(bi10->continue_for_header, 0u);
EXPECT_EQ(bi10->header_for_merge, 0u);
EXPECT_EQ(bi10->header_for_continue, 0u);
- EXPECT_FALSE(bi10->single_block_loop);
+ EXPECT_FALSE(bi10->is_single_block_loop);
// Single block loop is its own continue, and marked as single block loop.
const auto* bi20 = fe.GetBlockInfo(20);
@@ -447,7 +447,7 @@
EXPECT_EQ(bi20->continue_for_header, 20u);
EXPECT_EQ(bi20->header_for_merge, 0u);
EXPECT_EQ(bi20->header_for_continue, 20u);
- EXPECT_TRUE(bi20->single_block_loop);
+ EXPECT_TRUE(bi20->is_single_block_loop);
// Merge block points to the header
const auto* bi99 = fe.GetBlockInfo(99);
@@ -456,7 +456,7 @@
EXPECT_EQ(bi99->continue_for_header, 0u);
EXPECT_EQ(bi99->header_for_merge, 20u);
EXPECT_EQ(bi99->header_for_continue, 0u);
- EXPECT_FALSE(bi99->single_block_loop);
+ EXPECT_FALSE(bi99->is_single_block_loop);
}
TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) {
@@ -493,7 +493,7 @@
EXPECT_EQ(bi20->continue_for_header, 40u);
EXPECT_EQ(bi20->header_for_merge, 0u);
EXPECT_EQ(bi20->header_for_continue, 0u);
- EXPECT_FALSE(bi20->single_block_loop);
+ EXPECT_FALSE(bi20->is_single_block_loop);
// Continue block points to header
const auto* bi40 = fe.GetBlockInfo(40);
@@ -502,7 +502,7 @@
EXPECT_EQ(bi40->continue_for_header, 0u);
EXPECT_EQ(bi40->header_for_merge, 0u);
EXPECT_EQ(bi40->header_for_continue, 20u);
- EXPECT_FALSE(bi40->single_block_loop);
+ EXPECT_FALSE(bi40->is_single_block_loop);
// Merge block points to the header
const auto* bi99 = fe.GetBlockInfo(99);
@@ -511,7 +511,7 @@
EXPECT_EQ(bi99->continue_for_header, 0u);
EXPECT_EQ(bi99->header_for_merge, 20u);
EXPECT_EQ(bi99->header_for_continue, 0u);
- EXPECT_FALSE(bi99->single_block_loop);
+ EXPECT_FALSE(bi99->is_single_block_loop);
}
TEST_F(SpvParserTest,
@@ -549,7 +549,7 @@
EXPECT_EQ(bi20->continue_for_header, 40u);
EXPECT_EQ(bi20->header_for_merge, 0u);
EXPECT_EQ(bi20->header_for_continue, 0u);
- EXPECT_FALSE(bi20->single_block_loop);
+ EXPECT_FALSE(bi20->is_single_block_loop);
// Continue block points to header
const auto* bi40 = fe.GetBlockInfo(40);
@@ -558,7 +558,7 @@
EXPECT_EQ(bi40->continue_for_header, 0u);
EXPECT_EQ(bi40->header_for_merge, 0u);
EXPECT_EQ(bi40->header_for_continue, 20u);
- EXPECT_FALSE(bi40->single_block_loop);
+ EXPECT_FALSE(bi40->is_single_block_loop);
// Merge block points to the header
const auto* bi99 = fe.GetBlockInfo(99);
@@ -567,7 +567,7 @@
EXPECT_EQ(bi99->continue_for_header, 0u);
EXPECT_EQ(bi99->header_for_merge, 20u);
EXPECT_EQ(bi99->header_for_continue, 0u);
- EXPECT_FALSE(bi99->single_block_loop);
+ EXPECT_FALSE(bi99->is_single_block_loop);
}
TEST_F(SpvParserTest, RegisterMerges_SelectionMerge_BadTerminator) {
