diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index f9dd6b44c7..0d18d84fb1 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -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; } diff --git a/src/writer/spirv/builder_binary_expression_test.cc b/src/writer/spirv/builder_binary_expression_test.cc index 1b9c555d22..3a761c9798 100644 --- a/src/writer/spirv/builder_binary_expression_test.cc +++ b/src/writer/spirv/builder_binary_expression_test.cc @@ -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::BinaryOp::kLogicalAnd, + create( + create(&bool_ty, true)), + create( + create(&bool_ty, false))); + + ast::BinaryExpression expr(ast::BinaryOp::kLogicalOr, + create( + create(&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::BinaryOp::kLogicalOr, + create( + create(&bool_ty, true)), + create( + create(&bool_ty, false))); + + ast::BinaryExpression expr(ast::BinaryOp::kLogicalAnd, + create( + create(&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;