From a00a0a09ae5dab8ef15073ceb40542c162767798 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Fri, 27 Apr 2018 10:33:19 -0400 Subject: [PATCH] Revert "Improvements to vector dce." This reverts commit 2813722993d17a827497b8f627858e687adebbda. A regression was found. Undoing the change until it is fixed. --- source/opt/instruction.cpp | 77 --------- source/opt/instruction.h | 4 - source/opt/vector_dce.cpp | 150 +++++------------- source/opt/vector_dce.h | 27 +--- test/opt/dead_insert_elim_test.cpp | 118 ++++++++++++++ test/opt/vector_dce_test.cpp | 242 ++--------------------------- 6 files changed, 165 insertions(+), 453 deletions(-) diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 88553ad82..ba8413e35 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -596,82 +596,5 @@ bool Instruction::IsOpcodeCodeMotionSafe() const { } } -bool Instruction::IsScalarizable() const { - if (spvOpcodeIsScalarizable(opcode())) { - return true; - } - - const uint32_t kExtInstSetIdInIdx = 0; - const uint32_t kExtInstInstructionInIdx = 1; - - if (opcode() == SpvOpExtInst) { - uint32_t instSetId = - context()->get_feature_mgr()->GetExtInstImportId_GLSLstd450(); - - if (GetSingleWordInOperand(kExtInstSetIdInIdx) == instSetId) { - switch (GetSingleWordInOperand(kExtInstInstructionInIdx)) { - case GLSLstd450Round: - case GLSLstd450RoundEven: - case GLSLstd450Trunc: - case GLSLstd450FAbs: - case GLSLstd450SAbs: - case GLSLstd450FSign: - case GLSLstd450SSign: - case GLSLstd450Floor: - case GLSLstd450Ceil: - case GLSLstd450Fract: - case GLSLstd450Radians: - case GLSLstd450Degrees: - case GLSLstd450Sin: - case GLSLstd450Cos: - case GLSLstd450Tan: - case GLSLstd450Asin: - case GLSLstd450Acos: - case GLSLstd450Atan: - case GLSLstd450Sinh: - case GLSLstd450Cosh: - case GLSLstd450Tanh: - case GLSLstd450Asinh: - case GLSLstd450Acosh: - case GLSLstd450Atanh: - case GLSLstd450Atan2: - case GLSLstd450Pow: - case GLSLstd450Exp: - case GLSLstd450Log: - case GLSLstd450Exp2: - case GLSLstd450Log2: - case GLSLstd450Sqrt: - case GLSLstd450InverseSqrt: - case GLSLstd450Modf: - case GLSLstd450FMin: - case GLSLstd450UMin: - case GLSLstd450SMin: - case GLSLstd450FMax: - case GLSLstd450UMax: - case GLSLstd450SMax: - case GLSLstd450FClamp: - case GLSLstd450UClamp: - case GLSLstd450SClamp: - case GLSLstd450FMix: - case GLSLstd450Step: - case GLSLstd450SmoothStep: - case GLSLstd450Fma: - case GLSLstd450Frexp: - case GLSLstd450Ldexp: - case GLSLstd450FindILsb: - case GLSLstd450FindSMsb: - case GLSLstd450FindUMsb: - case GLSLstd450NMin: - case GLSLstd450NMax: - case GLSLstd450NClamp: - return true; - default: - return false; - } - } - } - return false; -} - } // namespace ir } // namespace spvtools diff --git a/source/opt/instruction.h b/source/opt/instruction.h index 1b05194ef..e74631057 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -24,7 +24,6 @@ #include "operand.h" #include "util/ilist_node.h" -#include "latest_version_glsl_std_450_header.h" #include "latest_version_spirv_header.h" #include "reflect.h" #include "spirv-tools/libspirv.h" @@ -406,8 +405,6 @@ class Instruction : public utils::IntrusiveNodeBase { // is always added to |options|. std::string PrettyPrint(uint32_t options = 0u) const; - bool IsScalarizable() const; - private: // Returns the total count of result type id and result id. uint32_t TypeResultIdCount() const { @@ -702,7 +699,6 @@ bool Instruction::IsAtomicOp() const { return spvOpcodeIsAtomicOp(opcode()); } bool Instruction::IsConstant() const { return IsCompileTimeConstantInst(opcode()); } - } // namespace ir } // namespace spvtools diff --git a/source/opt/vector_dce.cpp b/source/opt/vector_dce.cpp index ad5e3c60e..c8d11d50b 100644 --- a/source/opt/vector_dce.cpp +++ b/source/opt/vector_dce.cpp @@ -45,14 +45,27 @@ void VectorDCE::FindLiveComponents(ir::Function* function, // Prime the work list. We will assume that any instruction that does // not result in a vector is live. // + // TODO: If we want to remove all of the dead code with the current + // implementation, we will have to iterate VectorDCE and ADCE. That can be + // avoided if we treat scalar results as if they are vectors with 1 element. + // // Extending to structures and matrices is not as straight forward because of // the nesting. We cannot simply us a bit vector to keep track of which // components are live because of arbitrary nesting of structs. function->ForEachInst( [&work_list, this, live_components](ir::Instruction* current_inst) { - if (!HasVectorOrScalarResult(current_inst)) { - MarkUsesAsLive(current_inst, all_components_live_, live_components, - &work_list); + bool is_vector = HasVectorResult(current_inst); + if (!is_vector) { + switch (current_inst->opcode()) { + case SpvOpCompositeExtract: + MarkExtractUseAsLive(current_inst, live_components, &work_list); + break; + + default: + MarkUsesAsLive(current_inst, all_components_live_, + live_components, &work_list); + break; + } } }); @@ -62,9 +75,6 @@ void VectorDCE::FindLiveComponents(ir::Function* function, ir::Instruction* current_inst = current_item.instruction; switch (current_inst->opcode()) { - case SpvOpCompositeExtract: - MarkExtractUseAsLive(current_inst, live_components, &work_list); - break; case SpvOpCompositeInsert: MarkInsertUsesAsLive(current_item, live_components, &work_list); break; @@ -72,11 +82,13 @@ void VectorDCE::FindLiveComponents(ir::Function* function, MarkVectorShuffleUsesAsLive(current_item, live_components, &work_list); break; case SpvOpCompositeConstruct: - MarkCompositeContructUsesAsLive(current_item, live_components, - &work_list); + // TODO: If we have a vector parameter, then we can determine which + // of its components are live. + MarkUsesAsLive(current_inst, all_components_live_, live_components, + &work_list); break; default: - if (current_inst->IsScalarizable()) { + if (spvOpcodeIsScalarizable(current_inst->opcode())) { MarkUsesAsLive(current_inst, current_item.components, live_components, &work_list); } else { @@ -96,7 +108,7 @@ void VectorDCE::MarkExtractUseAsLive(const ir::Instruction* current_inst, current_inst->GetSingleWordInOperand(kExtractCompositeIdInIdx); ir::Instruction* operand_inst = def_use_mgr->GetDef(operand_id); - if (HasVectorOrScalarResult(operand_inst)) { + if (HasVectorResult(operand_inst)) { WorkListItem new_item; new_item.instruction = operand_inst; new_item.components.Set(current_inst->GetSingleWordInOperand(1)); @@ -157,46 +169,6 @@ void VectorDCE::MarkVectorShuffleUsesAsLive( AddItemToWorkListIfNeeded(second_operand, live_components, work_list); } -void VectorDCE::MarkCompositeContructUsesAsLive( - VectorDCE::WorkListItem work_item, - VectorDCE::LiveComponentMap* live_components, - std::vector* work_list) { - analysis::DefUseManager* def_use_mgr = context()->get_def_use_mgr(); - analysis::TypeManager* type_mgr = context()->get_type_mgr(); - - uint32_t current_component = 0; - ir::Instruction* current_inst = work_item.instruction; - uint32_t num_in_operands = current_inst->NumInOperands(); - for (uint32_t i = 0; i < num_in_operands; ++i) { - uint32_t id = current_inst->GetSingleWordInOperand(i); - ir::Instruction* op_inst = def_use_mgr->GetDef(id); - - if (HasScalarResult(op_inst)) { - WorkListItem new_work_item; - new_work_item.instruction = op_inst; - if (work_item.components.Get(current_component)) { - new_work_item.components.Set(0); - } - AddItemToWorkListIfNeeded(new_work_item, live_components, work_list); - current_component++; - } else { - assert(HasVectorResult(op_inst)); - WorkListItem new_work_item; - new_work_item.instruction = op_inst; - uint32_t op_vector_size = - type_mgr->GetType(op_inst->result_id())->AsVector()->element_count(); - - for (uint32_t op_vector_idx = 0; op_vector_idx < op_vector_size; - op_vector_idx++, current_component++) { - if (work_item.components.Get(current_component)) { - new_work_item.components.Set(op_vector_idx); - } - } - AddItemToWorkListIfNeeded(new_work_item, live_components, work_list); - } - } -} - void VectorDCE::MarkUsesAsLive( ir::Instruction* current_inst, const utils::BitVector& live_elements, LiveComponentMap* live_components, @@ -212,19 +184,10 @@ void VectorDCE::MarkUsesAsLive( new_item.instruction = operand_inst; new_item.components = live_elements; AddItemToWorkListIfNeeded(new_item, live_components, work_list); - } else if (HasScalarResult(operand_inst)) { - WorkListItem new_item; - new_item.instruction = operand_inst; - new_item.components.Set(0); - AddItemToWorkListIfNeeded(new_item, live_components, work_list); } }); } -bool VectorDCE::HasVectorOrScalarResult(const ir::Instruction* inst) const { - return HasScalarResult(inst) || HasVectorResult(inst); -} - bool VectorDCE::HasVectorResult(const ir::Instruction* inst) const { analysis::TypeManager* type_mgr = context()->get_type_mgr(); if (inst->type_id() == 0) { @@ -232,29 +195,8 @@ bool VectorDCE::HasVectorResult(const ir::Instruction* inst) const { } const analysis::Type* current_type = type_mgr->GetType(inst->type_id()); - switch (current_type->kind()) { - case analysis::Type::kVector: - return true; - default: - return false; - } -} - -bool VectorDCE::HasScalarResult(const ir::Instruction* inst) const { - analysis::TypeManager* type_mgr = context()->get_type_mgr(); - if (inst->type_id() == 0) { - return false; - } - - const analysis::Type* current_type = type_mgr->GetType(inst->type_id()); - switch (current_type->kind()) { - case analysis::Type::kBool: - case analysis::Type::kInteger: - case analysis::Type::kFloat: - return true; - default: - return false; - } + bool is_vector = current_type->AsVector() != nullptr; + return is_vector; } bool VectorDCE::RewriteInstructions( @@ -287,10 +229,19 @@ bool VectorDCE::RewriteInstructions( } switch (current_inst->opcode()) { - case SpvOpCompositeInsert: - modified |= - RewriteInsertInstruction(current_inst, live_component->second); - break; + case SpvOpCompositeInsert: { + // If the value being inserted is not live, then we can skip the + // insert. + uint32_t insert_index = current_inst->GetSingleWordInOperand(2); + if (!live_component->second.Get(insert_index)) { + modified = true; + context()->KillNamesAndDecorates(current_inst->result_id()); + uint32_t composite_id = + current_inst->GetSingleWordInOperand(kInsertCompositeIdInIdx); + context()->ReplaceAllUsesWith(current_inst->result_id(), + composite_id); + } + } break; case SpvOpCompositeConstruct: // TODO: The members that are not live can be replaced by an undef // or constant. This will remove uses of those values, and possibly @@ -304,33 +255,6 @@ bool VectorDCE::RewriteInstructions( return modified; } -bool VectorDCE::RewriteInsertInstruction( - ir::Instruction* current_inst, const utils::BitVector& live_components) { - // If the value being inserted is not live, then we can skip the insert. - bool modified = false; - uint32_t insert_index = current_inst->GetSingleWordInOperand(2); - if (!live_components.Get(insert_index)) { - modified = true; - context()->KillNamesAndDecorates(current_inst->result_id()); - uint32_t composite_id = - current_inst->GetSingleWordInOperand(kInsertCompositeIdInIdx); - context()->ReplaceAllUsesWith(current_inst->result_id(), composite_id); - } - - // If the values already in the composite are not used, then replace it with - // an undef. - utils::BitVector temp = live_components; - temp.Clear(insert_index); - if (temp.Empty()) { - context()->ForgetUses(current_inst); - uint32_t undef_id = Type2Undef(current_inst->type_id()); - current_inst->SetInOperand(kInsertCompositeIdInIdx, {undef_id}); - context()->AnalyzeUses(current_inst); - } - - return modified; -} - void VectorDCE::AddItemToWorkListIfNeeded( WorkListItem work_item, VectorDCE::LiveComponentMap* live_components, std::vector* work_list) { diff --git a/source/opt/vector_dce.h b/source/opt/vector_dce.h index 0e6aca6bc..85835b164 100644 --- a/source/opt/vector_dce.h +++ b/source/opt/vector_dce.h @@ -71,26 +71,8 @@ class VectorDCE : public MemPass { bool RewriteInstructions(ir::Function* function, const LiveComponentMap& live_components); - // Rewrites the OpCompositeInsert instruction |current_inst| to avoid - // unnecessary computes given that the only components of the result that are - // live are |live_components|. - // - // If the value being inserted is not live, then the result of |current_inst| - // is replaced by the composite input to |current_inst|. - // - // If the composite input to |current_inst| is not live, then it is replaced - // by and OpUndef in |current_inst|. - bool RewriteInsertInstruction(ir::Instruction* current_inst, - const utils::BitVector& live_components); - - // Returns true if the result of |inst| is a vector or a scalar. - bool HasVectorOrScalarResult(const ir::Instruction* inst) const; - - // Returns true if the result of |inst| is a scalar. - bool HasVectorResult(const ir::Instruction* inst) const; - // Returns true if the result of |inst| is a vector. - bool HasScalarResult(const ir::Instruction* inst) const; + bool HasVectorResult(const ir::Instruction* inst) const; // Adds |work_item| to |work_list| if it is not already live according to // |live_components|. |live_components| is updated to indicate that @@ -130,13 +112,6 @@ class VectorDCE : public MemPass { LiveComponentMap* live_components, std::vector* work_list); - // Marks the uses in the OpCompositeConstruct instruction |current_inst| as - // live. If anything becomes live they are added to |work_list| and - // |live_components| is updated accordingly. - void MarkCompositeContructUsesAsLive(WorkListItem work_item, - LiveComponentMap* live_components, - std::vector* work_list); - // A BitVector that can always be used to say that all components of a vector // are live. utils::BitVector all_components_live_; diff --git a/test/opt/dead_insert_elim_test.cpp b/test/opt/dead_insert_elim_test.cpp index 9b94c77ca..7a434363a 100644 --- a/test/opt/dead_insert_elim_test.cpp +++ b/test/opt/dead_insert_elim_test.cpp @@ -561,6 +561,124 @@ OpFunctionEnd before_predefs + before, after_predefs + after, true, true); } +TEST_F(DeadInsertElimTest, DeadInsertInCycleToDo) { + // Dead insert in chain with cycle. Demonstrates analysis can handle + // cycles in chains. + // + // TODO(greg-lunarg): Improve algorithm to remove dead insert into v.y. Will + // likely require similar logic to ADCE. + // + // Note: The SPIR-V assembly has had store/load elimination + // performed to allow the inserts and extracts to directly + // reference each other. + // + // #version 450 + // + // layout (location=0) in vec4 In0; + // layout (location=1) in float In1; + // layout (location=2) in float In2; + // layout (location=0) out vec4 OutColor; + // + // layout(std140, binding = 0 ) uniform _Globals_ + // { + // int g_n ; + // }; + // + // void main() + // { + // vec2 v = vec2(0.0, 1.0); + // for (int i = 0; i < g_n; i++) { + // v.x = v.x + 1; + // v.y = v.y * 0.9; // dead + // } + // OutColor = vec4(v.x); + // } + + const std::string assembly = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %OutColor %In0 %In1 %In2 +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 450 +OpName %main "main" +OpName %_Globals_ "_Globals_" +OpMemberName %_Globals_ 0 "g_n" +OpName %_ "" +OpName %OutColor "OutColor" +OpName %In0 "In0" +OpName %In1 "In1" +OpName %In2 "In2" +OpMemberDecorate %_Globals_ 0 Offset 0 +OpDecorate %_Globals_ Block +OpDecorate %_ DescriptorSet 0 +OpDecorate %_ Binding 0 +OpDecorate %OutColor Location 0 +OpDecorate %In0 Location 0 +OpDecorate %In1 Location 1 +OpDecorate %In2 Location 2 +%void = OpTypeVoid +%10 = OpTypeFunction %void +%float = OpTypeFloat 32 +%v2float = OpTypeVector %float 2 +%_ptr_Function_v2float = OpTypePointer Function %v2float +%float_0 = OpConstant %float 0 +%float_1 = OpConstant %float 1 +%16 = OpConstantComposite %v2float %float_0 %float_1 +%int = OpTypeInt 32 1 +%_ptr_Function_int = OpTypePointer Function %int +%int_0 = OpConstant %int 0 +%_Globals_ = OpTypeStruct %int +%_ptr_Uniform__Globals_ = OpTypePointer Uniform %_Globals_ +%_ = OpVariable %_ptr_Uniform__Globals_ Uniform +%_ptr_Uniform_int = OpTypePointer Uniform %int +%bool = OpTypeBool +%float_0_75 = OpConstant %float 0.75 +%int_1 = OpConstant %int 1 +%v4float = OpTypeVector %float 4 +%_ptr_Output_v4float = OpTypePointer Output %v4float +%OutColor = OpVariable %_ptr_Output_v4float Output +%_ptr_Input_v4float = OpTypePointer Input %v4float +%In0 = OpVariable %_ptr_Input_v4float Input +%_ptr_Input_float = OpTypePointer Input %float +%In1 = OpVariable %_ptr_Input_float Input +%In2 = OpVariable %_ptr_Input_float Input +%main = OpFunction %void None %10 +%29 = OpLabel +OpBranch %30 +%30 = OpLabel +%31 = OpPhi %v2float %16 %29 %32 %33 +%34 = OpPhi %int %int_0 %29 %35 %33 +OpLoopMerge %36 %33 None +OpBranch %37 +%37 = OpLabel +%38 = OpAccessChain %_ptr_Uniform_int %_ %int_0 +%39 = OpLoad %int %38 +%40 = OpSLessThan %bool %34 %39 +OpBranchConditional %40 %41 %36 +%41 = OpLabel +%42 = OpCompositeExtract %float %31 0 +%43 = OpFAdd %float %42 %float_1 +%44 = OpCompositeInsert %v2float %43 %31 0 +%45 = OpCompositeExtract %float %44 1 +%46 = OpFMul %float %45 %float_0_75 +%32 = OpCompositeInsert %v2float %46 %44 1 +OpBranch %33 +%33 = OpLabel +%35 = OpIAdd %int %34 %int_1 +OpBranch %30 +%36 = OpLabel +%47 = OpCompositeExtract %float %31 0 +%48 = OpCompositeConstruct %v4float %47 %47 %47 %47 +OpStore %OutColor %48 +OpReturn +OpFunctionEnd +)"; + + SinglePassRunAndCheck(assembly, assembly, true, + true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // diff --git a/test/opt/vector_dce_test.cpp b/test/opt/vector_dce_test.cpp index 573f5bc90..9eea34c79 100644 --- a/test/opt/vector_dce_test.cpp +++ b/test/opt/vector_dce_test.cpp @@ -354,225 +354,13 @@ OpFunctionEnd after_predefs + after, true, true); } -TEST_F(VectorDCETest, DeadInsertWithScalars) { - // Dead insert which requires two passes to eliminate - // - // Note: The SPIR-V assembly has had store/load elimination - // performed to allow the inserts and extracts to directly - // reference each other. - // - // #version 450 - // - // layout (location=0) in vec4 In0; - // layout (location=1) in float In1; - // layout (location=2) in float In2; - // layout (location=0) out vec4 OutColor; - // - // layout(std140, binding = 0 ) uniform _Globals_ - // { - // bool g_b; - // bool g_b2; - // }; - // - // void main() - // { - // vec4 v1, v2; - // v1 = In0; - // v1.y = In1 + In2; // dead, second pass - // if (g_b) v1.x = 1.0; - // v2.x = v1.x; - // v2.y = v1.y; // dead, first pass - // if (g_b2) v2.x = 0.0; - // OutColor = vec4(v2.x,v2.x,0.0,1.0); - // } - - const std::string before_predefs = - R"(OpCapability Shader -%1 = OpExtInstImport "GLSL.std.450" -OpMemoryModel Logical GLSL450 -OpEntryPoint Fragment %main "main" %In0 %In1 %In2 %OutColor -OpExecutionMode %main OriginUpperLeft -OpSource GLSL 450 -OpName %main "main" -OpName %In0 "In0" -OpName %In1 "In1" -OpName %In2 "In2" -OpName %_Globals_ "_Globals_" -OpMemberName %_Globals_ 0 "g_b" -OpMemberName %_Globals_ 1 "g_b2" -OpName %_ "" -OpName %OutColor "OutColor" -OpDecorate %In0 Location 0 -OpDecorate %In1 Location 1 -OpDecorate %In2 Location 2 -OpMemberDecorate %_Globals_ 0 Offset 0 -OpMemberDecorate %_Globals_ 1 Offset 4 -OpDecorate %_Globals_ Block -OpDecorate %_ DescriptorSet 0 -OpDecorate %_ Binding 0 -OpDecorate %OutColor Location 0 -%void = OpTypeVoid -%10 = OpTypeFunction %void -%float = OpTypeFloat 32 -%v4float = OpTypeVector %float 4 -%_ptr_Function_v4float = OpTypePointer Function %v4float -%_ptr_Input_v4float = OpTypePointer Input %v4float -%In0 = OpVariable %_ptr_Input_v4float Input -%_ptr_Input_float = OpTypePointer Input %float -%In1 = OpVariable %_ptr_Input_float Input -%In2 = OpVariable %_ptr_Input_float Input -%uint = OpTypeInt 32 0 -%_Globals_ = OpTypeStruct %uint %uint -%_ptr_Uniform__Globals_ = OpTypePointer Uniform %_Globals_ -%_ = OpVariable %_ptr_Uniform__Globals_ Uniform -%int = OpTypeInt 32 1 -%int_0 = OpConstant %int 0 -%_ptr_Uniform_uint = OpTypePointer Uniform %uint -%bool = OpTypeBool -%uint_0 = OpConstant %uint 0 -%float_1 = OpConstant %float 1 -%int_1 = OpConstant %int 1 -%float_0 = OpConstant %float 0 -%_ptr_Output_v4float = OpTypePointer Output %v4float -%OutColor = OpVariable %_ptr_Output_v4float Output -%27 = OpUndef %v4float -)"; - - const std::string after_predefs = - R"(OpCapability Shader -%1 = OpExtInstImport "GLSL.std.450" -OpMemoryModel Logical GLSL450 -OpEntryPoint Fragment %main "main" %In0 %In1 %In2 %OutColor -OpExecutionMode %main OriginUpperLeft -OpSource GLSL 450 -OpName %main "main" -OpName %In0 "In0" -OpName %In1 "In1" -OpName %In2 "In2" -OpName %_Globals_ "_Globals_" -OpMemberName %_Globals_ 0 "g_b" -OpMemberName %_Globals_ 1 "g_b2" -OpName %_ "" -OpName %OutColor "OutColor" -OpDecorate %In0 Location 0 -OpDecorate %In1 Location 1 -OpDecorate %In2 Location 2 -OpMemberDecorate %_Globals_ 0 Offset 0 -OpMemberDecorate %_Globals_ 1 Offset 4 -OpDecorate %_Globals_ Block -OpDecorate %_ DescriptorSet 0 -OpDecorate %_ Binding 0 -OpDecorate %OutColor Location 0 -%void = OpTypeVoid -%10 = OpTypeFunction %void -%float = OpTypeFloat 32 -%v4float = OpTypeVector %float 4 -%_ptr_Function_v4float = OpTypePointer Function %v4float -%_ptr_Input_v4float = OpTypePointer Input %v4float -%In0 = OpVariable %_ptr_Input_v4float Input -%_ptr_Input_float = OpTypePointer Input %float -%In1 = OpVariable %_ptr_Input_float Input -%In2 = OpVariable %_ptr_Input_float Input -%uint = OpTypeInt 32 0 -%_Globals_ = OpTypeStruct %uint %uint -%_ptr_Uniform__Globals_ = OpTypePointer Uniform %_Globals_ -%_ = OpVariable %_ptr_Uniform__Globals_ Uniform -%int = OpTypeInt 32 1 -%int_0 = OpConstant %int 0 -%_ptr_Uniform_uint = OpTypePointer Uniform %uint -%bool = OpTypeBool -%uint_0 = OpConstant %uint 0 -%float_1 = OpConstant %float 1 -%int_1 = OpConstant %int 1 -%float_0 = OpConstant %float 0 -%_ptr_Output_v4float = OpTypePointer Output %v4float -%OutColor = OpVariable %_ptr_Output_v4float Output -%27 = OpUndef %v4float -)"; - - const std::string before = - R"(%main = OpFunction %void None %10 -%28 = OpLabel -%29 = OpLoad %v4float %In0 -%30 = OpLoad %float %In1 -%31 = OpLoad %float %In2 -%32 = OpFAdd %float %30 %31 -%33 = OpCompositeInsert %v4float %32 %29 1 -%34 = OpAccessChain %_ptr_Uniform_uint %_ %int_0 -%35 = OpLoad %uint %34 -%36 = OpINotEqual %bool %35 %uint_0 -OpSelectionMerge %37 None -OpBranchConditional %36 %38 %37 -%38 = OpLabel -%39 = OpCompositeInsert %v4float %float_1 %33 0 -OpBranch %37 -%37 = OpLabel -%40 = OpPhi %v4float %33 %28 %39 %38 -%41 = OpCompositeExtract %float %40 0 -%42 = OpCompositeInsert %v4float %41 %27 0 -%43 = OpCompositeExtract %float %40 1 -%44 = OpCompositeInsert %v4float %43 %42 1 -%45 = OpAccessChain %_ptr_Uniform_uint %_ %int_1 -%46 = OpLoad %uint %45 -%47 = OpINotEqual %bool %46 %uint_0 -OpSelectionMerge %48 None -OpBranchConditional %47 %49 %48 -%49 = OpLabel -%50 = OpCompositeInsert %v4float %float_0 %44 0 -OpBranch %48 -%48 = OpLabel -%51 = OpPhi %v4float %44 %37 %50 %49 -%52 = OpCompositeExtract %float %51 0 -%53 = OpCompositeExtract %float %51 0 -%54 = OpCompositeConstruct %v4float %52 %53 %float_0 %float_1 -OpStore %OutColor %54 -OpReturn -OpFunctionEnd -)"; - - const std::string after = - R"(%main = OpFunction %void None %10 -%28 = OpLabel -%29 = OpLoad %v4float %In0 -%34 = OpAccessChain %_ptr_Uniform_uint %_ %int_0 -%35 = OpLoad %uint %34 -%36 = OpINotEqual %bool %35 %uint_0 -OpSelectionMerge %37 None -OpBranchConditional %36 %38 %37 -%38 = OpLabel -%39 = OpCompositeInsert %v4float %float_1 %29 0 -OpBranch %37 -%37 = OpLabel -%40 = OpPhi %v4float %29 %28 %39 %38 -%41 = OpCompositeExtract %float %40 0 -%42 = OpCompositeInsert %v4float %41 %27 0 -%45 = OpAccessChain %_ptr_Uniform_uint %_ %int_1 -%46 = OpLoad %uint %45 -%47 = OpINotEqual %bool %46 %uint_0 -OpSelectionMerge %48 None -OpBranchConditional %47 %49 %48 -%49 = OpLabel -%50 = OpCompositeInsert %v4float %float_0 %42 0 -OpBranch %48 -%48 = OpLabel -%51 = OpPhi %v4float %42 %37 %50 %49 -%52 = OpCompositeExtract %float %51 0 -%53 = OpCompositeExtract %float %51 0 -%54 = OpCompositeConstruct %v4float %52 %53 %float_0 %float_1 -OpStore %OutColor %54 -OpReturn -OpFunctionEnd -)"; - - SinglePassRunAndCheck( - before_predefs + before, after_predefs + after, true, true); -} - -#ifdef SPIRV_EFFCEE -TEST_F(VectorDCETest, DeadInsertInCycle) { +TEST_F(VectorDCETest, DeadInsertInCycleToDo) { // Dead insert in chain with cycle. Demonstrates analysis can handle // cycles in chains going through scalars intermediate values. // + // TODO: Improve algorithm to remove dead insert into v.y. Need to treat + // scalars at if they are vectors with a single element. + // // Note: The SPIR-V assembly has had store/load elimination // performed to allow the inserts and extracts to directly // reference each other. @@ -600,15 +388,7 @@ TEST_F(VectorDCETest, DeadInsertInCycle) { // } const std::string assembly = - R"( -; CHECK: [[init_val:%\w+]] = OpConstantComposite %v2float %float_0 %float_1 -; CHECK: [[undef:%\w+]] = OpUndef %v2float -; CHECK: OpFunction -; CHECK: [[entry_lab:%\w+]] = OpLabel -; CHECK: [[loop_header:%\w+]] = OpLabel -; CHECK: OpPhi %v2float [[init_val]] [[entry_lab]] [[x_insert:%\w+]] {{%\w+}} -; CHECK: [[x_insert:%\w+]] = OpCompositeInsert %v2float %43 [[undef]] 0 -OpCapability Shader + R"(OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %OutColor %In0 %In1 %In2 @@ -688,7 +468,7 @@ OpReturn OpFunctionEnd )"; - SinglePassRunAndMatch(assembly, true); + SinglePassRunAndCheck(assembly, assembly, true, true); } TEST_F(VectorDCETest, DeadLoadFeedingCompositeConstruct) { @@ -696,12 +476,7 @@ TEST_F(VectorDCETest, DeadLoadFeedingCompositeConstruct) { // TODO: Implement the rewrite for CompositeConstruct. const std::string assembly = - R"( -; CHECK: [[undef:%\w+]] = OpUndef %float -; CHECK: [[ac:%\w+]] = OpAccessChain %_ptr_Input_float %In0 %uint_2 -; CHECK: [[load:%\w+]] = OpLoad %float [[ac]] -; CHECK: OpCompositeConstruct %v3float [[load]] [[undef]] [[undef]] -OpCapability Shader + R"(OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %In0 %OutColor @@ -769,9 +544,10 @@ OpReturn OpFunctionEnd )"; - SinglePassRunAndMatch(assembly, true); + SinglePassRunAndCheck(assembly, assembly, true, true); } +#ifdef SPIRV_EFFCEE TEST_F(VectorDCETest, DeadLoadFeedingVectorShuffle) { // Detach the loads feeding the CompositeConstruct for the unused elements. // TODO: Implement the rewrite for CompositeConstruct.