mirror of
https://github.com/KhronosGroup/SPIRV-Tools
synced 2025-01-12 17:30:15 +00:00
Fix getting operand without checking opcode.
Fixes https://github.com/KhronosGhttps://github.com/KhronosGroup/SPIRV-Tools/issues/1559roup/SPIRV-Tools/issues/1559. There is an load of an operand of an instruction that was suppose to be only for the OpCompositeExtract case. However, an error caused it to be loaded for every opcode, even those that do not have an operand in that position. We fix up that bug, and a couple other things noticed that the same time.
This commit is contained in:
parent
efcc33e8a9
commit
0e1b7e5aef
@ -133,12 +133,13 @@ bool ReduceLoadSize::ShouldReplaceExtract(ir::Instruction* inst) {
|
||||
bool all_elements_used = false;
|
||||
std::set<uint32_t> elements_used;
|
||||
|
||||
def_use_mgr->ForEachUser(
|
||||
op_inst, [&elements_used, &all_elements_used](ir::Instruction* use) {
|
||||
all_elements_used = !def_use_mgr->WhileEachUser(
|
||||
op_inst, [&elements_used](ir::Instruction* use) {
|
||||
if (use->opcode() != SpvOpCompositeExtract) {
|
||||
all_elements_used = true;
|
||||
return false;
|
||||
}
|
||||
elements_used.insert(use->GetSingleWordInOperand(1));
|
||||
return true;
|
||||
});
|
||||
|
||||
bool should_replace = false;
|
||||
|
@ -114,7 +114,7 @@ TEST_F(ReduceLoadSizeTest, cbuffer_load_extract_vector) {
|
||||
// };
|
||||
//
|
||||
//
|
||||
// cbuffer gBuffer { uint a[32]; };
|
||||
// cbuffer gBuffer { uint4 a; };
|
||||
//
|
||||
// RWStructuredBuffer<S> gRWSBuffer;
|
||||
//
|
||||
@ -256,4 +256,69 @@ OpFunctionEnd
|
||||
SinglePassRunAndCheck<opt::ReduceLoadSize>(test, test, true, false);
|
||||
}
|
||||
|
||||
TEST_F(ReduceLoadSizeTest, cbuffer_load_fully_used) {
|
||||
// The result of the load (%22) is used in an instruction that uses the whole
|
||||
// load and has only 1 in operand. This trigger issue #1559.
|
||||
const std::string test =
|
||||
R"(OpCapability Shader
|
||||
OpMemoryModel Logical GLSL450
|
||||
OpEntryPoint GLCompute %main "main"
|
||||
OpExecutionMode %main LocalSize 1 1 1
|
||||
OpSource HLSL 600
|
||||
OpName %type_gBuffer "type.gBuffer"
|
||||
OpMemberName %type_gBuffer 0 "a"
|
||||
OpName %gBuffer "gBuffer"
|
||||
OpName %S "S"
|
||||
OpMemberName %S 0 "f"
|
||||
OpName %type_RWStructuredBuffer_S "type.RWStructuredBuffer.S"
|
||||
OpName %gRWSBuffer "gRWSBuffer"
|
||||
OpName %main "main"
|
||||
OpMemberDecorate %type_gBuffer 0 Offset 0
|
||||
OpDecorate %type_gBuffer Block
|
||||
OpMemberDecorate %S 0 Offset 0
|
||||
OpDecorate %_runtimearr_S ArrayStride 4
|
||||
OpMemberDecorate %type_RWStructuredBuffer_S 0 Offset 0
|
||||
OpDecorate %type_RWStructuredBuffer_S BufferBlock
|
||||
OpDecorate %gBuffer DescriptorSet 0
|
||||
OpDecorate %gBuffer Binding 0
|
||||
OpDecorate %gRWSBuffer DescriptorSet 0
|
||||
OpDecorate %gRWSBuffer Binding 1
|
||||
%uint = OpTypeInt 32 0
|
||||
%uint_32 = OpConstant %uint 32
|
||||
%v4uint = OpTypeVector %uint 4
|
||||
%float = OpTypeFloat 32
|
||||
%v4float = OpTypeVector %float 4
|
||||
%type_gBuffer = OpTypeStruct %v4uint
|
||||
%_ptr_Uniform_type_gBuffer = OpTypePointer Uniform %type_gBuffer
|
||||
%S = OpTypeStruct %uint
|
||||
%_runtimearr_S = OpTypeRuntimeArray %S
|
||||
%type_RWStructuredBuffer_S = OpTypeStruct %_runtimearr_S
|
||||
%_ptr_Uniform_type_RWStructuredBuffer_S = OpTypePointer Uniform %type_RWStructuredBuffer_S
|
||||
%int = OpTypeInt 32 1
|
||||
%void = OpTypeVoid
|
||||
%15 = OpTypeFunction %void
|
||||
%int_0 = OpConstant %int 0
|
||||
%_ptr_Uniform_v4uint = OpTypePointer Uniform %v4uint
|
||||
%uint_0 = OpConstant %uint 0
|
||||
%_ptr_Uniform_uint = OpTypePointer Uniform %uint
|
||||
%gBuffer = OpVariable %_ptr_Uniform_type_gBuffer Uniform
|
||||
%gRWSBuffer = OpVariable %_ptr_Uniform_type_RWStructuredBuffer_S Uniform
|
||||
%main = OpFunction %void None %15
|
||||
%20 = OpLabel
|
||||
%21 = OpAccessChain %_ptr_Uniform_v4uint %gBuffer %int_0
|
||||
%22 = OpLoad %v4uint %21
|
||||
%23 = OpCompositeExtract %uint %22 1
|
||||
%24 = OpConvertUToF %v4float %22
|
||||
%25 = OpAccessChain %_ptr_Uniform_uint %gRWSBuffer %int_0 %uint_0 %int_0
|
||||
OpStore %25 %23
|
||||
OpReturn
|
||||
OpFunctionEnd
|
||||
)";
|
||||
|
||||
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
|
||||
SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
|
||||
SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
|
||||
SinglePassRunAndCheck<opt::ReduceLoadSize>(test, test, true, false);
|
||||
}
|
||||
|
||||
} // anonymous namespace
|
||||
|
@ -272,9 +272,9 @@ Options (in lexicographical order):
|
||||
--private-to-local
|
||||
Change the scope of private variables that are used in a single
|
||||
function to that function.
|
||||
--remove-duplicates
|
||||
Removes duplicate types, decorations, capabilities and extension
|
||||
instructions.
|
||||
--reduce-load-size
|
||||
Replaces loads of composite objects where not every component is
|
||||
used by loads of just the elements that are used.
|
||||
--redundancy-elimination
|
||||
Looks for instructions in the same function that compute the
|
||||
same value, and deletes the redundant ones.
|
||||
@ -282,6 +282,9 @@ Options (in lexicographical order):
|
||||
Allow store from one struct type to a different type with
|
||||
compatible layout and members. This option is forwarded to the
|
||||
validator.
|
||||
--remove-duplicates
|
||||
Removes duplicate types, decorations, capabilities and extension
|
||||
instructions.
|
||||
--replace-invalid-opcode
|
||||
Replaces instructions whose opcode is valid for shader modules,
|
||||
but not for the current shader stage. To have an effect, all
|
||||
|
Loading…
Reference in New Issue
Block a user