diff --git a/source/opt/ir_builder.h b/source/opt/ir_builder.h index 98c7ed1cf..6720e8926 100644 --- a/source/opt/ir_builder.h +++ b/source/opt/ir_builder.h @@ -59,44 +59,27 @@ class InstructionBuilder { preserved_analyses) {} Instruction* AddNullaryOp(uint32_t type_id, SpvOp opcode) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } - std::unique_ptr new_inst( - new Instruction(GetContext(), opcode, type_id, result_id, {})); - return AddInstruction(std::move(new_inst)); + // TODO(1841): Handle id overflow. + std::unique_ptr newUnOp(new Instruction( + GetContext(), opcode, type_id, + opcode == SpvOpReturn ? 0 : GetContext()->TakeNextId(), {})); + return AddInstruction(std::move(newUnOp)); } Instruction* AddUnaryOp(uint32_t type_id, SpvOp opcode, uint32_t operand1) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } + // TODO(1841): Handle id overflow. std::unique_ptr newUnOp(new Instruction( - GetContext(), opcode, type_id, result_id, + GetContext(), opcode, type_id, GetContext()->TakeNextId(), {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand1}}})); return AddInstruction(std::move(newUnOp)); } Instruction* AddBinaryOp(uint32_t type_id, SpvOp opcode, uint32_t operand1, uint32_t operand2) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } + // TODO(1841): Handle id overflow. std::unique_ptr newBinOp(new Instruction( GetContext(), opcode, type_id, - opcode == SpvOpStore ? 0 : result_id, + opcode == SpvOpStore ? 0 : GetContext()->TakeNextId(), {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand1}}, {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand2}}})); return AddInstruction(std::move(newBinOp)); @@ -104,15 +87,9 @@ class InstructionBuilder { Instruction* AddTernaryOp(uint32_t type_id, SpvOp opcode, uint32_t operand1, uint32_t operand2, uint32_t operand3) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } + // TODO(1841): Handle id overflow. std::unique_ptr newTernOp(new Instruction( - GetContext(), opcode, type_id, result_id, + GetContext(), opcode, type_id, GetContext()->TakeNextId(), {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand1}}, {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand2}}, {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand3}}})); @@ -122,15 +99,9 @@ class InstructionBuilder { Instruction* AddQuadOp(uint32_t type_id, SpvOp opcode, uint32_t operand1, uint32_t operand2, uint32_t operand3, uint32_t operand4) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } + // TODO(1841): Handle id overflow. std::unique_ptr newQuadOp(new Instruction( - GetContext(), opcode, type_id, result_id, + GetContext(), opcode, type_id, GetContext()->TakeNextId(), {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand1}}, {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand2}}, {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {operand3}}, @@ -140,15 +111,9 @@ class InstructionBuilder { Instruction* AddIdLiteralOp(uint32_t type_id, SpvOp opcode, uint32_t id, uint32_t uliteral) { - uint32_t result_id = 0; - if (type_id != 0) { - result_id = GetContext()->TakeNextId(); - if (result_id == 0) { - return nullptr; - } - } + // TODO(1841): Handle id overflow. std::unique_ptr newBinOp(new Instruction( - GetContext(), opcode, type_id, result_id, + GetContext(), opcode, type_id, GetContext()->TakeNextId(), {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {id}}, {spv_operand_type_t::SPV_OPERAND_TYPE_LITERAL_INTEGER, {uliteral}}})); return AddInstruction(std::move(newBinOp)); diff --git a/source/opt/wrap_opkill.cpp b/source/opt/wrap_opkill.cpp index fb32178e8..eb63bd7c3 100644 --- a/source/opt/wrap_opkill.cpp +++ b/source/opt/wrap_opkill.cpp @@ -59,14 +59,7 @@ bool WrapOpKill::ReplaceWithFunctionCall(Instruction* inst) { if (ir_builder.AddFunctionCall(GetVoidTypeId(), func_id, {}) == nullptr) { return false; } - - uint32_t return_type_id = GetOwningFunctionsReturnType(inst); - if (return_type_id != GetVoidTypeId()) { - Instruction* undef = ir_builder.AddNullaryOp(return_type_id, SpvOpUndef); - ir_builder.AddUnaryOp(0, SpvOpReturnValue, undef->result_id()); - } else { - ir_builder.AddNullaryOp(0, SpvOpReturn); - } + ir_builder.AddUnreachable(); context()->KillInst(inst); return true; } @@ -154,15 +147,5 @@ uint32_t WrapOpKill::GetOpKillFuncId() { return opkill_function_->result_id(); } -uint32_t WrapOpKill::GetOwningFunctionsReturnType(Instruction* inst) { - BasicBlock* bb = context()->get_instr_block(inst); - if (bb == nullptr) { - return 0; - } - - Function* func = bb->GetParent(); - return func->type_id(); -} - } // namespace opt } // namespace spvtools diff --git a/source/opt/wrap_opkill.h b/source/opt/wrap_opkill.h index 87a5d692c..8b0328132 100644 --- a/source/opt/wrap_opkill.h +++ b/source/opt/wrap_opkill.h @@ -56,10 +56,6 @@ class WrapOpKill : public Pass { // function could not be generated. uint32_t GetOpKillFuncId(); - // Returns the id of the return type for the function that contains |inst|. - // Returns 0 if |inst| is not in a function. - uint32_t GetOwningFunctionsReturnType(Instruction* inst); - // The id of the void type. If its value is 0, then the void type has not // been found or created yet. uint32_t void_type_id_; diff --git a/test/opt/wrap_opkill_test.cpp b/test/opt/wrap_opkill_test.cpp index 7e502be1f..7162cc563 100644 --- a/test/opt/wrap_opkill_test.cpp +++ b/test/opt/wrap_opkill_test.cpp @@ -31,7 +31,7 @@ TEST_F(WrapOpKillTest, SingleOpKill) { ; CHECK: [[orig_kill]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpFunctionCall %void [[new_kill:%\w+]] -; CHECK-NEXT: OpReturn +; CHECK-NEXT: OpUnreachable ; CHECK: [[new_kill]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpKill @@ -83,10 +83,10 @@ TEST_F(WrapOpKillTest, MultipleOpKillInSameFunc) { ; CHECK-NEXT: OpBranchConditional ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpFunctionCall %void [[new_kill:%\w+]] -; CHECK-NEXT: OpReturn +; CHECK-NEXT: OpUnreachable ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpFunctionCall %void [[new_kill]] -; CHECK-NEXT: OpReturn +; CHECK-NEXT: OpUnreachable ; CHECK: [[new_kill]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpKill @@ -143,11 +143,11 @@ TEST_F(WrapOpKillTest, MultipleOpKillInDifferentFunc) { ; CHECK: [[orig_kill1]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpFunctionCall %void [[new_kill:%\w+]] -; CHECK-NEXT: OpReturn +; CHECK-NEXT: OpUnreachable ; CHECK: [[orig_kill2]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpFunctionCall %void [[new_kill]] -; CHECK-NEXT: OpReturn +; CHECK-NEXT: OpUnreachable ; CHECK: [[new_kill]] = OpFunction ; CHECK-NEXT: OpLabel ; CHECK-NEXT: OpKill @@ -193,58 +193,6 @@ TEST_F(WrapOpKillTest, MultipleOpKillInDifferentFunc) { SinglePassRunAndMatch(text, true); } -TEST_F(WrapOpKillTest, FuncWithReturnValue) { - const std::string text = R"( -; CHECK: OpEntryPoint Fragment [[main:%\w+]] -; CHECK: [[main]] = OpFunction -; CHECK: OpFunctionCall %int [[orig_kill:%\w+]] -; CHECK: [[orig_kill]] = OpFunction -; CHECK-NEXT: OpLabel -; CHECK-NEXT: OpFunctionCall %void [[new_kill:%\w+]] -; CHECK-NEXT: [[undef:%\w+]] = OpUndef %int -; CHECK-NEXT: OpReturnValue [[undef]] -; CHECK: [[new_kill]] = OpFunction -; CHECK-NEXT: OpLabel -; CHECK-NEXT: OpKill -; CHECK-NEXT: OpFunctionEnd - OpCapability Shader - %1 = OpExtInstImport "GLSL.std.450" - OpMemoryModel Logical GLSL450 - OpEntryPoint Fragment %main "main" - OpExecutionMode %main OriginUpperLeft - OpSource GLSL 330 - OpName %main "main" - %void = OpTypeVoid - %5 = OpTypeFunction %void - %int = OpTypeInt 32 1 - %func_type = OpTypeFunction %int - %bool = OpTypeBool - %true = OpConstantTrue %bool - %main = OpFunction %void None %5 - %8 = OpLabel - OpBranch %9 - %9 = OpLabel - OpLoopMerge %10 %11 None - OpBranch %12 - %12 = OpLabel - OpBranchConditional %true %13 %10 - %13 = OpLabel - OpBranch %11 - %11 = OpLabel - %14 = OpFunctionCall %int %kill_ - OpBranch %9 - %10 = OpLabel - OpReturn - OpFunctionEnd - %kill_ = OpFunction %int None %func_type - %15 = OpLabel - OpKill - OpFunctionEnd - )"; - - SinglePassRunAndMatch(text, true); -} - TEST_F(WrapOpKillTest, IdBoundOverflow1) { const std::string text = R"( OpCapability GeometryStreams