[spirv-reader] Uses in phis count as uses
This is required so we actually emit values that are only used in phis. Bug: tint:3, tint:215 Change-Id: I9d957a697839b8d09246905c3f28064f0bc01731 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/27701 Commit-Queue: David Neto <dneto@google.com> Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
parent
c5d65cc917
commit
5a75a174d6
|
@ -3244,27 +3244,25 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
|
|||
const auto* block_info = GetBlockInfo(block_id);
|
||||
const auto block_pos = block_info->pos;
|
||||
for (const auto& inst : *(block_info->basic_block)) {
|
||||
// Update the usage span for IDs used by this instruction.
|
||||
// But skip uses in OpPhi because they are handled differently.
|
||||
if (inst.opcode() != SpvOpPhi) {
|
||||
inst.ForEachInId([this, block_pos, block_info](const uint32_t* id_ptr) {
|
||||
auto* def_info = GetDefInfo(*id_ptr);
|
||||
if (def_info) {
|
||||
def_info->num_uses++;
|
||||
def_info->last_use_pos =
|
||||
std::max(def_info->last_use_pos, block_pos);
|
||||
// Update bookkeeping for locally-defined IDs used by this instruction.
|
||||
inst.ForEachInId([this, block_pos, block_info](const uint32_t* id_ptr) {
|
||||
auto* def_info = GetDefInfo(*id_ptr);
|
||||
if (def_info) {
|
||||
// Update usage count.
|
||||
def_info->num_uses++;
|
||||
// Update usage span.
|
||||
def_info->last_use_pos = std::max(def_info->last_use_pos, block_pos);
|
||||
|
||||
// Determine whether this ID is defined in a different construct
|
||||
// from this use.
|
||||
const auto defining_block = block_order_[def_info->block_pos];
|
||||
const auto* def_in_construct =
|
||||
GetBlockInfo(defining_block)->construct;
|
||||
if (def_in_construct != block_info->construct) {
|
||||
def_info->used_in_another_construct = true;
|
||||
}
|
||||
// Determine whether this ID is defined in a different construct
|
||||
// from this use.
|
||||
const auto defining_block = block_order_[def_info->block_pos];
|
||||
const auto* def_in_construct =
|
||||
GetBlockInfo(defining_block)->construct;
|
||||
if (def_in_construct != block_info->construct) {
|
||||
def_info->used_in_another_construct = true;
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
if (inst.opcode() == SpvOpPhi) {
|
||||
// Declare a name for the variable used to carry values to a phi.
|
||||
|
|
|
@ -1363,23 +1363,13 @@ TEST_F(SpvParserTest, EmitStatement_Phi_SingleBlockLoopIndex) {
|
|||
}
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_4
|
||||
none
|
||||
__u32
|
||||
{
|
||||
Binary{
|
||||
Identifier{x_2}
|
||||
add
|
||||
ScalarConstructor{1}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
Identifier{x_2_phi}
|
||||
Identifier{x_4}
|
||||
Binary{
|
||||
Identifier{x_2}
|
||||
add
|
||||
ScalarConstructor{1}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
Identifier{x_3_phi}
|
||||
|
@ -1495,6 +1485,13 @@ TEST_F(SpvParserTest, EmitStatement_Phi_MultiBlockLoopIndex) {
|
|||
}
|
||||
}
|
||||
Loop{
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_4
|
||||
function
|
||||
__u32
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_2
|
||||
|
@ -1524,18 +1521,12 @@ TEST_F(SpvParserTest, EmitStatement_Phi_MultiBlockLoopIndex) {
|
|||
}
|
||||
}
|
||||
continuing {
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_4
|
||||
none
|
||||
__u32
|
||||
{
|
||||
Binary{
|
||||
Identifier{x_2}
|
||||
add
|
||||
ScalarConstructor{1}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
Identifier{x_4}
|
||||
Binary{
|
||||
Identifier{x_2}
|
||||
add
|
||||
ScalarConstructor{1}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
|
@ -1632,6 +1623,13 @@ Loop{
|
|||
ScalarConstructor{1}
|
||||
}
|
||||
Loop{
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_7
|
||||
function
|
||||
__u32
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_2
|
||||
|
@ -1689,18 +1687,12 @@ Loop{
|
|||
}
|
||||
}
|
||||
continuing {
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_7
|
||||
none
|
||||
__u32
|
||||
{
|
||||
Binary{
|
||||
Identifier{x_4}
|
||||
add
|
||||
Identifier{x_6}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
Identifier{x_7}
|
||||
Binary{
|
||||
Identifier{x_4}
|
||||
add
|
||||
Identifier{x_6}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
|
@ -1957,6 +1949,103 @@ Return{}
|
|||
)")) << ToString(fe.ast_body());
|
||||
}
|
||||
|
||||
TEST_F(SpvParserTest, EmitStatement_UseInPhiCountsAsUse) {
|
||||
// From crbug.com/215
|
||||
// If the only use of a combinatorially computed ID is as the value
|
||||
// in an OpPhi, then we still have to emit it. The algorithm fix
|
||||
// is to always count uses in Phis.
|
||||
// This is the reduced case from the bug report.
|
||||
//
|
||||
// The only use of %12 is in the phi.
|
||||
// The only use of %11 is in %12.
|
||||
// Both definintions need to be emitted to the output.
|
||||
auto assembly = Preamble() + R"(
|
||||
%100 = OpFunction %void None %voidfn
|
||||
|
||||
%10 = OpLabel
|
||||
%11 = OpLogicalAnd %bool %true %true
|
||||
%12 = OpLogicalNot %bool %11 ;
|
||||
OpSelectionMerge %99 None
|
||||
OpBranchConditional %true %20 %99
|
||||
|
||||
%20 = OpLabel
|
||||
OpBranch %99
|
||||
|
||||
%99 = OpLabel
|
||||
%101 = OpPhi %bool %11 %10 %12 %20
|
||||
OpReturn
|
||||
|
||||
OpFunctionEnd
|
||||
|
||||
)";
|
||||
auto* p = parser(test::Assemble(assembly));
|
||||
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << assembly;
|
||||
FunctionEmitter fe(p, *spirv_function(100));
|
||||
EXPECT_TRUE(fe.EmitBody()) << p->error();
|
||||
|
||||
EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(VariableDeclStatement{
|
||||
Variable{
|
||||
x_101_phi
|
||||
function
|
||||
__bool
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_11
|
||||
none
|
||||
__bool
|
||||
{
|
||||
Binary{
|
||||
ScalarConstructor{true}
|
||||
logical_and
|
||||
ScalarConstructor{true}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_12
|
||||
none
|
||||
__bool
|
||||
{
|
||||
UnaryOp{
|
||||
not
|
||||
Identifier{x_11}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Assignment{
|
||||
Identifier{x_101_phi}
|
||||
Identifier{x_11}
|
||||
}
|
||||
If{
|
||||
(
|
||||
ScalarConstructor{true}
|
||||
)
|
||||
{
|
||||
Assignment{
|
||||
Identifier{x_101_phi}
|
||||
Identifier{x_12}
|
||||
}
|
||||
}
|
||||
}
|
||||
VariableDeclStatement{
|
||||
Variable{
|
||||
x_101
|
||||
none
|
||||
__bool
|
||||
{
|
||||
Identifier{x_101_phi}
|
||||
}
|
||||
}
|
||||
}
|
||||
Return{}
|
||||
)")) << ToString(fe.ast_body());
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace spirv
|
||||
} // namespace reader
|
||||
|
|
Loading…
Reference in New Issue