encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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>
This commit is contained in:
David Neto 2020-04-23 20:28:34 +00:00
parent a82384ee97
commit 742790c8ff
3 changed files with 26 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/reader/spirv/function.cc
View File

@ -390,7 +390,8 @@ bool FunctionEmitter::RegisterMerges() {
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 @@ bool FunctionEmitter::RegisterMerges() {
}
// 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";

4
src/reader/spirv/function.h
View File

@ -64,7 +64,7 @@ struct BlockInfo {
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 @@ class FunctionEmitter {
/// @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.

32
src/reader/spirv/function_cfg_test.cc
View File

@ -309,7 +309,7 @@ TEST_F(SpvParserTest, RegisterMerges_NoMerges) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) {
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 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) {
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 @@ TEST_F(SpvParserTest,
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 @@ TEST_F(SpvParserTest,
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 @@ TEST_F(SpvParserTest,
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) {