Skip to content

Commit 148a9fe

Browse files
s-perrons-perron
authored
Merge pull request KhronosGroup#6302 from s-perron/id_overflow4
spirv-opt: Handle id overflow in descriptor array access pass
2 parents bc98001 + d93e394 commit 148a9fe

15 files changed

+176
-71
lines changed

‎source/opt/amd_ext_to_khr.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ bool ReplaceSwizzleInvocations(IRContext* ctx, Instruction* inst,
232232
ctx->get_def_use_mgr()->GetDef(var_inst->type_id());
233233
uint32_t uint_type_id = var_ptr_type->GetSingleWordInOperand(1);
234234

235+
// TODO(1841): Handle id overflow.
235236
Instruction* id = ir_builder.AddLoad(uint_type_id, var_id);
236237

237238
uint32_t quad_mask = ir_builder.GetUintConstantId(3);
@@ -264,9 +265,11 @@ bool ReplaceSwizzleInvocations(IRContext* ctx, Instruction* inst,
264265
{uint_max_id, uint_max_id, uint_max_id, uint_max_id});
265266
Instruction* ballot_value =
266267
const_mgr->GetDefiningInstruction(ballot_value_const);
268+
// TODO(1841): Handle id overflow.
267269
Instruction* is_active = ir_builder.AddNaryOp(
268270
type_mgr->GetBoolTypeId(), spv::Op::OpGroupNonUniformBallotBitExtract,
269271
{subgroup_scope, ballot_value->result_id(), target_inv->result_id()});
272+
// TODO(1841): Handle id overflow.
270273
Instruction* shuffle =
271274
ir_builder.AddNaryOp(inst->type_id(), spv::Op::OpGroupNonUniformShuffle,
272275
{subgroup_scope, data_id, target_inv->result_id()});
@@ -358,6 +361,7 @@ bool ReplaceSwizzleInvocationsMasked(
358361
ctx->get_def_use_mgr()->GetDef(var_inst->type_id());
359362
uint32_t uint_type_id = var_ptr_type->GetSingleWordInOperand(1);
360363

364+
// TODO(1841): Handle id overflow.
361365
Instruction* id = ir_builder.AddLoad(uint_type_id, var_id);
362366

363367
// Do the bitwise operations.
@@ -381,9 +385,11 @@ bool ReplaceSwizzleInvocationsMasked(
381385
{uint_max_id, uint_max_id, uint_max_id, uint_max_id});
382386
Instruction* ballot_value =
383387
const_mgr->GetDefiningInstruction(ballot_value_const);
388+
// TODO(1841): Handle id overflow.
384389
Instruction* is_active = ir_builder.AddNaryOp(
385390
type_mgr->GetBoolTypeId(), spv::Op::OpGroupNonUniformBallotBitExtract,
386391
{subgroup_scope, ballot_value->result_id(), target_inv->result_id()});
392+
// TODO(1841): Handle id overflow.
387393
Instruction* shuffle =
388394
ir_builder.AddNaryOp(inst->type_id(), spv::Op::OpGroupNonUniformShuffle,
389395
{subgroup_scope, data_id, target_inv->result_id()});
@@ -435,6 +441,7 @@ bool ReplaceWriteInvocation(IRContext* ctx, Instruction* inst,
435441
InstructionBuilder ir_builder(
436442
ctx, inst,
437443
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
444+
// TODO(1841): Handle id overflow.
438445
Instruction* t =
439446
ir_builder.AddLoad(var_ptr_type->GetSingleWordInOperand(1), var_id);
440447
analysis::Bool bool_type;
@@ -598,8 +605,11 @@ bool ReplaceCubeFaceCoord(IRContext* ctx, Instruction* inst,
598605
const_mgr->GetDefiningInstruction(vec_const)->result_id();
599606

600607
// Extract the input values.
608+
// TODO(1841): Handle id overflow.
601609
Instruction* x = ir_builder.AddCompositeExtract(float_type_id, input_id, {0});
610+
// TODO(1841): Handle id overflow.
602611
Instruction* y = ir_builder.AddCompositeExtract(float_type_id, input_id, {1});
612+
// TODO(1-841): Handle id overflow.
603613
Instruction* z = ir_builder.AddCompositeExtract(float_type_id, input_id, {2});
604614

605615
// Negate the input values.
@@ -650,21 +660,27 @@ bool ReplaceCubeFaceCoord(IRContext* ctx, Instruction* inst,
650660
not_is_z_max->result_id(), y_gr_x->result_id());
651661

652662
// Select the correct value for cubesc.
663+
// TODO(1841): Handle id overflow.
653664
Instruction* cubesc_case_1 = ir_builder.AddSelect(
654665
float_type_id, is_z_neg->result_id(), nx->result_id(), x->result_id());
666+
// TODO(1841): Handle id overflow.
655667
Instruction* cubesc_case_2 = ir_builder.AddSelect(
656668
float_type_id, is_x_neg->result_id(), z->result_id(), nz->result_id());
657669
Instruction* sel =
670+
// TODO(1841): Handle id overflow.
658671
ir_builder.AddSelect(float_type_id, is_y_max->result_id(), x->result_id(),
659672
cubesc_case_2->result_id());
660673
Instruction* cubesc =
674+
// TODO(1841): Handle id overflow.
661675
ir_builder.AddSelect(float_type_id, is_z_max->result_id(),
662676
cubesc_case_1->result_id(), sel->result_id());
663677

664678
// Select the correct value for cubetc.
679+
// TODO(1841): Handle id overflow.
665680
Instruction* cubetc_case_1 = ir_builder.AddSelect(
666681
float_type_id, is_y_neg->result_id(), nz->result_id(), z->result_id());
667682
Instruction* cubetc =
683+
// TODO(1841): Handle id overflow.
668684
ir_builder.AddSelect(float_type_id, is_y_max->result_id(),
669685
cubetc_case_1->result_id(), ny->result_id());
670686

@@ -746,8 +762,11 @@ bool ReplaceCubeFaceIndex(IRContext* ctx, Instruction* inst,
746762
uint32_t f5_const_id = const_mgr->GetFloatConstId(5.0);
747763

748764
// Extract the input values.
765+
// TODO(1841): Handle id overflow.
749766
Instruction* x = ir_builder.AddCompositeExtract(float_type_id, input_id, {0});
767+
// TODO(1841): Handle id overflow.
750768
Instruction* y = ir_builder.AddCompositeExtract(float_type_id, input_id, {1});
769+
// TODO(1-841): Handle id overflow.
751770
Instruction* z = ir_builder.AddCompositeExtract(float_type_id, input_id, {2});
752771

753772
// Get the absolute values of the inputs.
@@ -778,15 +797,19 @@ bool ReplaceCubeFaceIndex(IRContext* ctx, Instruction* inst,
778797
ay->result_id(), ax->result_id());
779798

780799
// Get the value for each case.
800+
// TODO(1841): Handle id overflow.
781801
Instruction* case_z = ir_builder.AddSelect(
782802
float_type_id, is_z_neg->result_id(), f5_const_id, f4_const_id);
803+
// TODO(1841): Handle id overflow.
783804
Instruction* case_y = ir_builder.AddSelect(
784805
float_type_id, is_y_neg->result_id(), f3_const_id, f2_const_id);
806+
// TODO(1841): Handle id overflow.
785807
Instruction* case_x = ir_builder.AddSelect(
786808
float_type_id, is_x_neg->result_id(), f1_const_id, f0_const_id);
787809

788810
// Select the correct case.
789811
Instruction* sel =
812+
// TODO(1841): Handle id overflow.
790813
ir_builder.AddSelect(float_type_id, y_gr_x->result_id(),
791814
case_y->result_id(), case_x->result_id());
792815

‎source/opt/cfg.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ BasicBlock* CFG::SplitLoopHeader(BasicBlock* bb) {
279279
context, &*bb->begin(),
280280
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
281281

282+
// TODO(1841): Handle id overflow.
282283
Instruction* new_phi = builder.AddPhi(phi->type_id(), preheader_phi_ops);
283284

284285
// Add the OpPhi to the header bb.

‎source/opt/combine_access_chains.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ bool CombineAccessChains::CombineIndices(Instruction* ptr_input,
158158
InstructionBuilder builder(
159159
context(), inst,
160160
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
161+
// TODO(1841): Handle id overflow.
161162
Instruction* addition = builder.AddIAdd(last_index_inst->type_id(),
162163
last_index_inst->result_id(),
163164
element_inst->result_id());

‎source/opt/fix_func_call_arguments.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,19 @@ uint32_t FixFuncCallArgumentsPass::ReplaceAccessChainFuncCallArguments(
7474
op_type->result_id(), spv::StorageClass::Function);
7575
// Create new variable
7676
builder.SetInsertPoint(variable_insertion_point);
77+
// TODO(1841): Handle id overflow.
7778
Instruction* var =
7879
builder.AddVariable(varType, uint32_t(spv::StorageClass::Function));
7980
// Load access chain to the new variable before function call
8081
builder.SetInsertPoint(func_call_inst);
8182

8283
uint32_t operand_id = operand_inst->result_id();
84+
// TODO(1841): Handle id overflow.
8385
Instruction* load = builder.AddLoad(op_type->result_id(), operand_id);
8486
builder.AddStore(var->result_id(), load->result_id());
8587
// Load return value to the acesschain after function call
8688
builder.SetInsertPoint(next_insert_point);
89+
// TODO(1841): Handle id overflow.
8790
load = builder.AddLoad(op_type->result_id(), var->result_id());
8891
builder.AddStore(operand_id, load->result_id());
8992

‎source/opt/if_conversion.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ Pass::Status IfConversion::Process() {
126126
condition = SplatCondition(vec_data_ty, condition, &builder);
127127
}
128128

129+
// TODO(1841): Handle id overflow.
129130
Instruction* select = builder.AddSelect(phi->type_id(), condition,
130131
true_value->result_id(),
131132
false_value->result_id());
@@ -205,6 +206,7 @@ uint32_t IfConversion::SplatCondition(analysis::Vector* vec_data_ty,
205206
uint32_t bool_vec_id =
206207
context()->get_type_mgr()->GetTypeInstruction(&bool_vec_ty);
207208
std::vector<uint32_t> ids(vec_data_ty->element_count(), cond);
209+
// TODO(1841): Handle id overflow.
208210
return builder->AddCompositeConstruct(bool_vec_id, ids)->result_id();
209211
}
210212

‎source/opt/ir_builder.h‎

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,14 @@ class InstructionBuilder {
166166
for (size_t i = 0; i < operands.size(); i++) {
167167
ops.push_back({SPV_OPERAND_TYPE_ID, {operands[i]}});
168168
}
169-
// TODO(1841): Handle id overflow.
170-
std::unique_ptr<Instruction> new_inst(new Instruction(
171-
GetContext(), opcode, type_id,
172-
result != 0 ? result : GetContext()->TakeNextId(), ops));
169+
if (result == 0) {
170+
result = GetContext()->TakeNextId();
171+
if (result == 0) {
172+
return nullptr;
173+
}
174+
}
175+
std::unique_ptr<Instruction> new_inst(
176+
new Instruction(GetContext(), opcode, type_id, result, ops));
173177
return AddInstruction(std::move(new_inst));
174178
}
175179

@@ -297,9 +301,12 @@ class InstructionBuilder {
297301
// The id |op1| is the left hand side of the operation.
298302
// The id |op2| is the right hand side of the operation.
299303
Instruction* AddIAdd(uint32_t type, uint32_t op1, uint32_t op2) {
300-
// TODO(1841): Handle id overflow.
304+
uint32_t result_id = GetContext()->TakeNextId();
305+
if (result_id == 0) {
306+
return nullptr;
307+
}
301308
std::unique_ptr<Instruction> inst(new Instruction(
302-
GetContext(), spv::Op::OpIAdd, type, GetContext()->TakeNextId(),
309+
GetContext(), spv::Op::OpIAdd, type, result_id,
303310
{{SPV_OPERAND_TYPE_ID, {op1}}, {SPV_OPERAND_TYPE_ID, {op2}}}));
304311
return AddInstruction(std::move(inst));
305312
}
@@ -311,9 +318,12 @@ class InstructionBuilder {
311318
Instruction* AddULessThan(uint32_t op1, uint32_t op2) {
312319
analysis::Bool bool_type;
313320
uint32_t type = GetContext()->get_type_mgr()->GetId(&bool_type);
314-
// TODO(1841): Handle id overflow.
321+
uint32_t result_id = GetContext()->TakeNextId();
322+
if (result_id == 0) {
323+
return nullptr;
324+
}
315325
std::unique_ptr<Instruction> inst(new Instruction(
316-
GetContext(), spv::Op::OpULessThan, type, GetContext()->TakeNextId(),
326+
GetContext(), spv::Op::OpULessThan, type, result_id,
317327
{{SPV_OPERAND_TYPE_ID, {op1}}, {SPV_OPERAND_TYPE_ID, {op2}}}));
318328
return AddInstruction(std::move(inst));
319329
}
@@ -325,9 +335,12 @@ class InstructionBuilder {
325335
Instruction* AddSLessThan(uint32_t op1, uint32_t op2) {
326336
analysis::Bool bool_type;
327337
uint32_t type = GetContext()->get_type_mgr()->GetId(&bool_type);
328-
// TODO(1841): Handle id overflow.
338+
uint32_t result_id = GetContext()->TakeNextId();
339+
if (result_id == 0) {
340+
return nullptr;
341+
}
329342
std::unique_ptr<Instruction> inst(new Instruction(
330-
GetContext(), spv::Op::OpSLessThan, type, GetContext()->TakeNextId(),
343+
GetContext(), spv::Op::OpSLessThan, type, result_id,
331344
{{SPV_OPERAND_TYPE_ID, {op1}}, {SPV_OPERAND_TYPE_ID, {op2}}}));
332345
return AddInstruction(std::move(inst));
333346
}
@@ -355,9 +368,12 @@ class InstructionBuilder {
355368
// bool) for |type|.
356369
Instruction* AddSelect(uint32_t type, uint32_t cond, uint32_t true_value,
357370
uint32_t false_value) {
358-
// TODO(1841): Handle id overflow.
371+
uint32_t result_id = GetContext()->TakeNextId();
372+
if (result_id == 0) {
373+
return nullptr;
374+
}
359375
std::unique_ptr<Instruction> select(new Instruction(
360-
GetContext(), spv::Op::OpSelect, type, GetContext()->TakeNextId(),
376+
GetContext(), spv::Op::OpSelect, type, result_id,
361377
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {cond}},
362378
{SPV_OPERAND_TYPE_ID, {true_value}},
363379
{SPV_OPERAND_TYPE_ID, {false_value}}}));
@@ -381,10 +397,12 @@ class InstructionBuilder {
381397
ops.emplace_back(SPV_OPERAND_TYPE_ID,
382398
std::initializer_list<uint32_t>{id});
383399
}
384-
// TODO(1841): Handle id overflow.
385-
std::unique_ptr<Instruction> construct(
386-
new Instruction(GetContext(), spv::Op::OpCompositeConstruct, type,
387-
GetContext()->TakeNextId(), ops));
400+
uint32_t result_id = GetContext()->TakeNextId();
401+
if (result_id == 0) {
402+
return nullptr;
403+
}
404+
std::unique_ptr<Instruction> construct(new Instruction(
405+
GetContext(), spv::Op::OpCompositeConstruct, type, result_id, ops));
388406
return AddInstruction(std::move(construct));
389407
}
390408

@@ -466,10 +484,12 @@ class InstructionBuilder {
466484
operands.push_back({SPV_OPERAND_TYPE_LITERAL_INTEGER, {index}});
467485
}
468486

469-
// TODO(1841): Handle id overflow.
470-
std::unique_ptr<Instruction> new_inst(
471-
new Instruction(GetContext(), spv::Op::OpCompositeExtract, type,
472-
GetContext()->TakeNextId(), operands));
487+
uint32_t result_id = GetContext()->TakeNextId();
488+
if (result_id == 0) {
489+
return nullptr;
490+
}
491+
std::unique_ptr<Instruction> new_inst(new Instruction(
492+
GetContext(), spv::Op::OpCompositeExtract, type, result_id, operands));
473493
return AddInstruction(std::move(new_inst));
474494
}
475495

@@ -493,9 +513,12 @@ class InstructionBuilder {
493513
operands.push_back({SPV_OPERAND_TYPE_ID, {index_id}});
494514
}
495515

496-
// TODO(1841): Handle id overflow.
497-
std::unique_ptr<Instruction> new_inst(new Instruction(
498-
GetContext(), opcode, type_id, GetContext()->TakeNextId(), operands));
516+
uint32_t result_id = GetContext()->TakeNextId();
517+
if (result_id == 0) {
518+
return nullptr;
519+
}
520+
std::unique_ptr<Instruction> new_inst(
521+
new Instruction(GetContext(), opcode, type_id, result_id, operands));
499522
return AddInstruction(std::move(new_inst));
500523
}
501524

@@ -521,29 +544,36 @@ class InstructionBuilder {
521544
operands.push_back({SPV_OPERAND_TYPE_TYPED_LITERAL_NUMBER, {alignment}});
522545
}
523546

524-
// TODO(1841): Handle id overflow.
525-
std::unique_ptr<Instruction> new_inst(
526-
new Instruction(GetContext(), spv::Op::OpLoad, type_id,
527-
GetContext()->TakeNextId(), operands));
547+
uint32_t result_id = GetContext()->TakeNextId();
548+
if (result_id == 0) {
549+
return nullptr;
550+
}
551+
std::unique_ptr<Instruction> new_inst(new Instruction(
552+
GetContext(), spv::Op::OpLoad, type_id, result_id, operands));
528553
return AddInstruction(std::move(new_inst));
529554
}
530555

531556
Instruction* AddCopyObject(uint32_t type_id, uint32_t value_id) {
532557
std::vector<Operand> operands{{SPV_OPERAND_TYPE_ID, {value_id}}};
533558

534-
// TODO(1841): Handle id overflow.
535-
std::unique_ptr<Instruction> new_inst(
536-
new Instruction(GetContext(), spv::Op::OpCopyObject, type_id,
537-
GetContext()->TakeNextId(), operands));
559+
uint32_t result_id = GetContext()->TakeNextId();
560+
if (result_id == 0) {
561+
return nullptr;
562+
}
563+
std::unique_ptr<Instruction> new_inst(new Instruction(
564+
GetContext(), spv::Op::OpCopyObject, type_id, result_id, operands));
538565
return AddInstruction(std::move(new_inst));
539566
}
540567

541568
Instruction* AddVariable(uint32_t type_id, uint32_t storage_class) {
542569
std::vector<Operand> operands;
543570
operands.push_back({SPV_OPERAND_TYPE_STORAGE_CLASS, {storage_class}});
544-
std::unique_ptr<Instruction> new_inst(
545-
new Instruction(GetContext(), spv::Op::OpVariable, type_id,
546-
GetContext()->TakeNextId(), operands));
571+
uint32_t result_id = GetContext()->TakeNextId();
572+
if (result_id == 0) {
573+
return nullptr;
574+
}
575+
std::unique_ptr<Instruction> new_inst(new Instruction(
576+
GetContext(), spv::Op::OpVariable, type_id, result_id, operands));
547577
return AddInstruction(std::move(new_inst));
548578
}
549579

‎source/opt/loop_peeling.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ bool LoopPeeling::InsertCanonicalInductionVariable(
181181
// Create the increment.
182182
// Note that we do "1 + 1" here, one of the operand should the phi
183183
// value but we don't have it yet. The operand will be set latter.
184+
// TODO(1841): Handle id overflow.
184185
Instruction* iv_inc = builder.AddIAdd(
185186
uint_1_cst->type_id(), uint_1_cst->result_id(), uint_1_cst->result_id());
186187
if (!iv_inc) return false;
@@ -458,6 +459,7 @@ bool LoopPeeling::PeelBefore(uint32_t peel_factor) {
458459
builder.GetIntConstant(peel_factor, int_type_->IsSigned());
459460
if (!factor) return false;
460461

462+
// TODO(1841): Handle id overflow.
461463
Instruction* has_remaining_iteration = builder.AddLessThan(
462464
factor->result_id(), loop_iteration_count_->result_id());
463465
if (!has_remaining_iteration) return false;
@@ -534,6 +536,7 @@ bool LoopPeeling::PeelAfter(uint32_t peel_factor) {
534536
builder.GetIntConstant(peel_factor, int_type_->IsSigned());
535537
if (!factor) return false;
536538

539+
// TODO(1841): Handle id overflow.
537540
Instruction* has_remaining_iteration = builder.AddLessThan(
538541
factor->result_id(), loop_iteration_count_->result_id());
539542
if (!has_remaining_iteration) return false;

‎source/opt/loop_utils.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ class LCSSARewriter {
118118
}
119119
InstructionBuilder builder(base_->context_, &*bb->begin(),
120120
IRContext::kAnalysisInstrToBlockMapping);
121+
// TODO(1841): Handle id overflow.
121122
Instruction* incoming_phi =
122123
builder.AddPhi(def_insn_.type_id(), incomings);
123124

@@ -137,6 +138,7 @@ class LCSSARewriter {
137138
}
138139
InstructionBuilder builder(base_->context_, &*bb->begin(),
139140
IRContext::kAnalysisInstrToBlockMapping);
141+
// TODO(1841): Handle id overflow.
140142
Instruction* incoming_phi =
141143
builder.AddPhi(def_insn_.type_id(), incomings);
142144

@@ -399,6 +401,7 @@ void LoopUtils::CreateLoopDedicatedExits() {
399401
}
400402

401403
// Build the new phi instruction dedicated exit block.
404+
// TODO(1841): Handle id overflow.
402405
Instruction* exit_phi = builder.AddPhi(phi->type_id(), exit_phi_op);
403406
// Build the new incoming branch.
404407
new_phi_op.push_back(exit_phi->result_id());

0 commit comments

Comments
 (0)