spirv-reader: emit null for value from unreachable block
Sometimes a value can come from a block which does not appear in the structured block order. That block will never execute, so we can safely use the null value instead. Bug: tint:804 Change-Id: Idc1a6c4f76f14a1d76919a95e5a65e56018ce68f Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52840 Kokoro: Kokoro <noreply+kokoro@google.com> Reviewed-by: James Price <jrprice@google.com> Commit-Queue: James Price <jrprice@google.com> Auto-Submit: David Neto <dneto@google.com>
This commit is contained in:
parent
e8177dade6
commit
2871ad9138
|
@ -1321,6 +1321,9 @@ void FunctionEmitter::ComputeBlockOrderAndPositions() {
|
|||
for (uint32_t i = 0; i < block_order_.size(); ++i) {
|
||||
GetBlockInfo(block_order_[i])->pos = i;
|
||||
}
|
||||
// The invalid block position is not the position of any block that is in the
|
||||
// order.
|
||||
assert(block_order_.size() <= kInvalidBlockPos);
|
||||
}
|
||||
|
||||
bool FunctionEmitter::VerifyHeaderContinueMergeOrder() {
|
||||
|
@ -2294,6 +2297,17 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) {
|
|||
default:
|
||||
break;
|
||||
}
|
||||
if (const spvtools::opt::BasicBlock* const bb =
|
||||
ir_context_.get_instr_block(id)) {
|
||||
if (auto* block = GetBlockInfo(bb->id())) {
|
||||
if (block->pos == kInvalidBlockPos) {
|
||||
// The value came from a block not in the block order.
|
||||
// Substitute a null value.
|
||||
return parser_impl_.MakeNullExpression(
|
||||
parser_impl_.ConvertType(inst->type_id()));
|
||||
}
|
||||
}
|
||||
}
|
||||
Fail() << "unhandled expression for ID " << id << "\n" << inst->PrettyPrint();
|
||||
return {};
|
||||
}
|
||||
|
|
|
@ -70,6 +70,8 @@ enum class EdgeKind {
|
|||
kForward
|
||||
};
|
||||
|
||||
enum : uint32_t { kInvalidBlockPos = ~(0u) };
|
||||
|
||||
/// Bookkeeping info for a basic block.
|
||||
struct BlockInfo {
|
||||
/// Constructor
|
||||
|
@ -84,7 +86,8 @@ struct BlockInfo {
|
|||
uint32_t id = 0;
|
||||
|
||||
/// The position of this block in the reverse structured post-order.
|
||||
uint32_t pos = 0;
|
||||
/// If the block is not in that order, then this remains the invalid value.
|
||||
uint32_t pos = kInvalidBlockPos;
|
||||
|
||||
/// If this block is a header, then this is the ID of the merge block.
|
||||
uint32_t merge_for_header = 0;
|
||||
|
|
|
@ -496,6 +496,63 @@ INSTANTIATE_TEST_SUITE_P(
|
|||
{4, "", "vector component index is larger than 3: 4"},
|
||||
{99999, "", "vector component index is larger than 3: 99999"}}));
|
||||
|
||||
TEST_F(SpvParserTest, ValueFromBlockNotInBlockOrder) {
|
||||
// crbug.com/tint/804
|
||||
const auto assembly = Preamble() + CommonTypes() + R"(
|
||||
%float_42 = OpConstant %float 42.0
|
||||
%cond = OpUndef %bool
|
||||
|
||||
%100 = OpFunction %void None %voidfn
|
||||
%10 = OpLabel
|
||||
OpBranch %30
|
||||
|
||||
; unreachable
|
||||
%20 = OpLabel
|
||||
%499 = OpFAdd %float %float_42 %float_42
|
||||
%500 = OpFAdd %float %499 %float_42
|
||||
OpBranch %25
|
||||
|
||||
%25 = OpLabel
|
||||
OpBranch %80
|
||||
|
||||
|
||||
%30 = OpLabel
|
||||
OpLoopMerge %90 %80 None
|
||||
OpBranchConditional %cond %90 %40
|
||||
|
||||
%40 = OpLabel
|
||||
OpBranch %90
|
||||
|
||||
%80 = OpLabel ; unreachable continue target
|
||||
; but "dominated" by %20 and %25
|
||||
%81 = OpFMul %float %500 %float_42 ; %500 is defined in %20
|
||||
OpBranch %30 ; backedge
|
||||
|
||||
%90 = OpLabel
|
||||
OpReturn
|
||||
OpFunctionEnd
|
||||
)";
|
||||
auto p = parser(test::Assemble(assembly));
|
||||
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
|
||||
auto fe = p->function_emitter(100);
|
||||
EXPECT_TRUE(fe.EmitBody()) << p->error();
|
||||
const auto got = ToString(p->builder(), fe.ast_body());
|
||||
EXPECT_THAT(got, HasSubstr(R"(VariableDeclStatement{
|
||||
VariableConst{
|
||||
x_81
|
||||
none
|
||||
__f32
|
||||
{
|
||||
Binary[not set]{
|
||||
ScalarConstructor[not set]{0.000000}
|
||||
multiply
|
||||
ScalarConstructor[not set]{42.000000}
|
||||
}
|
||||
}
|
||||
}
|
||||
})"));
|
||||
}
|
||||
|
||||
// TODO(dneto): OpSizeof : requires Kernel (OpenCL)
|
||||
|
||||
} // namespace
|
||||
|
|
Loading…
Reference in New Issue