diff options
author | Steven Perron <31666470+s-perron@users.noreply.github.com> | 2018-08-15 09:14:21 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-15 09:14:21 -0400 |
commit | e065cc208f499f906301f92c6e9e51881b3943e9 (patch) | |
tree | bc766ae432f81a8473abce8c0248c9a407435003 /test/opt/local_single_store_elim_test.cpp | |
parent | ef678672fb04ee396c1c8de755f714f1c3d65267 (diff) | |
download | spirv-tools-e065cc208f499f906301f92c6e9e51881b3943e9.tar.gz |
Keep decorations when replacing loads in access-chain-convert. (#1829)
In local-access-chain-convert, we replace loads by load the entire
variable, then doing the extract. The extract will have the same value
as the load. However, if the load has a decoration on it, the
decoration is lost because we do not copy any them to the new id.
This is fixed by rewritting the load into the extract and keeping the
same result id.
This change has the effect that we do not call DCEInst on the loads
because the load is not being deleted, but replaced. This could leave
OpAccessChain instructions around that are not used. This is not a
problem for -O and -Os. They run local_single_*_elim passes and then
dead code elimination. The dce will remove the unused access chains,
and the load elimination passes work even if there are unused access
chains. I have added test to them to ensure they will not loss
opportunities.
Fixes #1787.
Diffstat (limited to 'test/opt/local_single_store_elim_test.cpp')
-rw-r--r-- | test/opt/local_single_store_elim_test.cpp | 74 |
1 files changed, 74 insertions, 0 deletions
diff --git a/test/opt/local_single_store_elim_test.cpp b/test/opt/local_single_store_elim_test.cpp index 51bd5d29..23e82ba8 100644 --- a/test/opt/local_single_store_elim_test.cpp +++ b/test/opt/local_single_store_elim_test.cpp @@ -771,6 +771,80 @@ OpFunctionEnd SinglePassRunAndCheck<LocalSingleStoreElimPass>(before, after, true, true); } +// Test that that an unused OpAccessChain between a store and a use does does +// not hinders the replacement of the use. We need to check this because +// local-access-chain-convert does always remove the OpAccessChain instructions +// that become dead. + +TEST_F(LocalSingleStoreElimTest, + StoreElimWithUnusedInterveningAccessChainLoad) { + // Last load of v is eliminated, but access chain load and store of v isn't + // + // #version 140 + // + // in vec4 BaseColor; + // + // void main() + // { + // vec4 v = BaseColor; + // float f = v[3]; + // gl_FragColor = v * f; + // } + + const std::string predefs = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %BaseColor %gl_FragColor +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 140 +OpName %main "main" +OpName %v "v" +OpName %BaseColor "BaseColor" +OpName %gl_FragColor "gl_FragColor" +%void = OpTypeVoid +%8 = OpTypeFunction %void +%float = OpTypeFloat 32 +%v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float +%_ptr_Input_v4float = OpTypePointer Input %v4float +%BaseColor = OpVariable %_ptr_Input_v4float Input +%_ptr_Function_float = OpTypePointer Function %float +%uint = OpTypeInt 32 0 +%uint_3 = OpConstant %uint 3 +%_ptr_Output_v4float = OpTypePointer Output %v4float +%gl_FragColor = OpVariable %_ptr_Output_v4float Output +)"; + + const std::string before = + R"(%main = OpFunction %void None %8 +%17 = OpLabel +%v = OpVariable %_ptr_Function_v4float Function +%18 = OpLoad %v4float %BaseColor +OpStore %v %18 +%19 = OpAccessChain %_ptr_Function_float %v %uint_3 +%21 = OpLoad %v4float %v +OpStore %gl_FragColor %21 +OpReturn +OpFunctionEnd +)"; + + const std::string after = + R"(%main = OpFunction %void None %8 +%17 = OpLabel +%v = OpVariable %_ptr_Function_v4float Function +%18 = OpLoad %v4float %BaseColor +OpStore %v %18 +%19 = OpAccessChain %_ptr_Function_float %v %uint_3 +OpStore %gl_FragColor %18 +OpReturn +OpFunctionEnd +)"; + + SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); + SinglePassRunAndCheck<LocalSingleStoreElimPass>(predefs + before, + predefs + after, true, true); +} // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Other types |