Skip to content

Commit

Permalink
[opt] Fix bug opt::InstructionBuilder::AddVariable (#6007)
Browse files Browse the repository at this point in the history
The storage class operand was being added with the wrong
operand type.  It was assumed to be an ID.  Then trying
to analyze the new variable declaration instruction's
defs and uses could get confused and assert out.

Added a test for this.
  • Loading branch information
dneto0 authored Feb 22, 2025
1 parent f2dac2f commit aafd524
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion source/opt/ir_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ class InstructionBuilder {

Instruction* AddVariable(uint32_t type_id, uint32_t storage_class) {
std::vector<Operand> operands;
operands.push_back({SPV_OPERAND_TYPE_ID, {storage_class}});
operands.push_back({SPV_OPERAND_TYPE_STORAGE_CLASS, {storage_class}});
std::unique_ptr<Instruction> new_inst(
new Instruction(GetContext(), spv::Op::OpVariable, type_id,
GetContext()->TakeNextId(), operands));
Expand Down
32 changes: 32 additions & 0 deletions test/opt/ir_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,38 @@ OpFunctionEnd
Match(text, context.get());
}

TEST_F(IRBuilderTest, AddVariable) {
// Use Private beacuse its' enun is 7 which is higher
// than the ID limit.
const std::string text = R"(
; CHECK: [[uint:%\w+]] = OpTypeInt 32 0
; CHECK: [[ptr:%\w+]] = OpTypePointer Private [[uint]]
; CHECK: [[var:%\w+]] = OpVariable [[ptr]] Private
; CHECK: OpTypeFloat
OpCapability Kernel
OpCapability VectorComputeINTEL
OpCapability Linkage
OpExtension "SPV_INTEL_vector_compute"
OpMemoryModel Logical OpenCL
%1 = OpTypeInt 32 0
%2 = OpTypePointer Private %1
%3 = OpTypeFloat 32
)";

std::unique_ptr<IRContext> context =
BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
EXPECT_NE(nullptr, context) << text;

auto* float_ty = context->get_def_use_mgr()->GetDef(3);
InstructionBuilder builder(context.get(), float_ty);
auto* var = builder.AddVariable(2u, uint32_t(spv::StorageClass::Private));
EXPECT_NE(nullptr, var);

context->get_def_use_mgr()->AnalyzeInstDefUse(var); // should not assert

Match(text, context.get());
}

TEST_F(IRBuilderTest, AddCompositeConstruct) {
const std::string text = R"(
; CHECK: [[uint:%\w+]] = OpTypeInt
Expand Down

0 comments on commit aafd524

Please sign in to comment.