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

spirv-writer: Fix phi for short-circuiting operators 

The Phi in the merge block was taking the value of the RHS
from the wrong basic block ID. Instead of taking it from
the first block of the expression for the RHS, take it from
the last block of the expression for the RHS.

Bug: tint:355
Change-Id: I1b79a1b107459fd420e39963ad7ab2e89bc4494f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33640
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
David Neto 2020-11-23 17:17:35 +00:00 committed by Commit Bot service account
parent 6cd6f7462c
commit 4c32dd9735
2 changed files with 98 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/writer/spirv/builder.cc
View File

@ -1481,6 +1481,8 @@ uint32_t Builder::GenerateShortCircuitBinaryExpression(
}
lhs_id = GenerateLoadIfNeeded(expr->lhs()->result_type(), lhs_id);
// Get the ID of the basic block where control flow will diverge. It's the
// last basic block generated for the left-hand-side of the operator.
auto original_label_id = current_label_id_;
auto type_id = GenerateTypeIfNeeded(expr->result_type());
@ -1517,6 +1519,9 @@ uint32_t Builder::GenerateShortCircuitBinaryExpression(
}
rhs_id = GenerateLoadIfNeeded(expr->rhs()->result_type(), rhs_id);
// Get the block ID of the last basic block generated for the right-hand-side
// expression. That block will be an immediate predecessor to the merge block.
auto rhs_block_id = current_label_id_;
push_function_inst(spv::Op::OpBranch, {Operand::Int(merge_block_id)});
// Output the merge block
@ -1528,7 +1533,7 @@ uint32_t Builder::GenerateShortCircuitBinaryExpression(
push_function_inst(spv::Op::OpPhi,
{Operand::Int(type_id), result, Operand::Int(lhs_id),
Operand::Int(original_label_id), Operand::Int(rhs_id),
Operand::Int(block_id)});
Operand::Int(rhs_block_id)});
return result_id;
}

92
src/writer/spirv/builder_binary_expression_test.cc
View File

@ -903,6 +903,98 @@ OpBranch %9
)");
}
TEST_F(BuilderTest, Binary_logicalOr_Nested_LogicalAnd) {
ast::type::BoolType bool_ty;
// Test an expression like
// a || (b && c)
// From: crbug.com/tint/355
auto* logical_and_expr = create<ast::BinaryExpression>(
ast::BinaryOp::kLogicalAnd,
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, true)),
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, false)));
ast::BinaryExpression expr(ast::BinaryOp::kLogicalOr,
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, true)),
logical_and_expr);
ASSERT_TRUE(td.DetermineResultType(&expr)) << td.error();
b.push_function(Function{});
b.GenerateLabel(b.next_id());
EXPECT_EQ(b.GenerateBinaryExpression(&expr), 10u) << b.error();
EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeBool
%3 = OpConstantTrue %2
%8 = OpConstantFalse %2
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
R"(%1 = OpLabel
OpSelectionMerge %4 None
OpBranchConditional %3 %4 %5
%5 = OpLabel
OpSelectionMerge %6 None
OpBranchConditional %3 %7 %6
%7 = OpLabel
OpBranch %6
%6 = OpLabel
%9 = OpPhi %2 %3 %5 %8 %7
OpBranch %4
%4 = OpLabel
%10 = OpPhi %2 %3 %1 %9 %6
)");
}
TEST_F(BuilderTest, Binary_logicalAnd_Nested_LogicalOr) {
ast::type::BoolType bool_ty;
// Test an expression like
// a && (b || c)
// From: crbug.com/tint/355
auto* logical_or_expr = create<ast::BinaryExpression>(
ast::BinaryOp::kLogicalOr,
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, true)),
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, false)));
ast::BinaryExpression expr(ast::BinaryOp::kLogicalAnd,
create<ast::ScalarConstructorExpression>(
create<ast::BoolLiteral>(&bool_ty, true)),
logical_or_expr);
ASSERT_TRUE(td.DetermineResultType(&expr)) << td.error();
b.push_function(Function{});
b.GenerateLabel(b.next_id());
EXPECT_EQ(b.GenerateBinaryExpression(&expr), 10u) << b.error();
EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeBool
%3 = OpConstantTrue %2
%8 = OpConstantFalse %2
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
R"(%1 = OpLabel
OpSelectionMerge %4 None
OpBranchConditional %3 %5 %4
%5 = OpLabel
OpSelectionMerge %6 None
OpBranchConditional %3 %6 %7
%7 = OpLabel
OpBranch %6
%6 = OpLabel
%9 = OpPhi %2 %3 %5 %8 %7
OpBranch %4
%4 = OpLabel
%10 = OpPhi %2 %3 %1 %9 %6
)");
}
TEST_F(BuilderTest, Binary_LogicalOr) {
ast::type::I32Type i32;