Skip to content

Commit 973c29e

Browse files
ConvertUBOToPushConstantPass: minor refactor
- Improve const-correctness - Use unordered_set for seen - Use references instead of non-null pointers
1 parent 1980a5d commit 973c29e

File tree

1 file changed

+29
-25
lines changed

1 file changed

+29
-25
lines changed

Graphics/ShaderTools/src/ConvertUBOToPushConstant.cpp

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
# pragma warning(pop)
3838
#endif
3939

40+
#include <vector>
41+
#include <unordered_set>
42+
4043
namespace Diligent
4144
{
4245

@@ -231,15 +234,15 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
231234
modified = true;
232235

233236
// Propagate storage class change to all users of this variable
234-
std::set<uint32_t> seen;
235237
std::vector<spvtools::opt::Instruction*> users;
236238
get_def_use_mgr()->ForEachUser(target_var, [&users](spvtools::opt::Instruction* user) {
237239
users.push_back(user);
238240
});
239241

242+
std::unordered_set<uint32_t> seen;
240243
for (spvtools::opt::Instruction* user : users)
241244
{
242-
modified |= PropagateStorageClass(user, &seen);
245+
modified |= PropagateStorageClass(*user, seen);
243246
}
244247

245248
// Remove Binding and DescriptorSet decorations from the variable
@@ -267,7 +270,7 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
267270
private:
268271
// Recursively updates the storage class of pointer types used by instructions
269272
// that reference the target variable.
270-
bool PropagateStorageClass(spvtools::opt::Instruction* inst, std::set<uint32_t>* seen)
273+
bool PropagateStorageClass(spvtools::opt::Instruction& inst, std::unordered_set<uint32_t>& seen) const
271274
{
272275
if (!IsPointerResultType(inst))
273276
{
@@ -277,35 +280,36 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
277280
// Already has the correct storage class
278281
if (IsPointerToStorageClass(inst, spv::StorageClass::PushConstant))
279282
{
280-
if (inst->opcode() == spv::Op::OpPhi)
283+
if (inst.opcode() == spv::Op::OpPhi)
281284
{
282-
if (!seen->insert(inst->result_id()).second)
285+
if (!seen.insert(inst.result_id()).second)
283286
{
284287
return false;
285288
}
286289
}
287290

288291
bool modified = false;
289292
std::vector<spvtools::opt::Instruction*> users;
290-
get_def_use_mgr()->ForEachUser(inst, [&users](spvtools::opt::Instruction* user) {
293+
get_def_use_mgr()->ForEachUser(&inst, [&users](spvtools::opt::Instruction* user) {
291294
users.push_back(user);
292295
});
293296
for (spvtools::opt::Instruction* user : users)
294297
{
295-
modified |= PropagateStorageClass(user, seen);
298+
if (PropagateStorageClass(*user, seen))
299+
modified = true;
296300
}
297301

298-
if (inst->opcode() == spv::Op::OpPhi)
302+
if (inst.opcode() == spv::Op::OpPhi)
299303
{
300-
seen->erase(inst->result_id());
304+
seen.erase(inst.result_id());
301305
}
302306
return modified;
303307
}
304308

305309
// Handle instructions that produce pointer results
306310
// This switch covers the common pointer-producing opcodes.
307311
// Reference: SPIRV-Tools fix_storage_class.cpp
308-
switch (inst->opcode())
312+
switch (inst.opcode())
309313
{
310314
case spv::Op::OpAccessChain:
311315
case spv::Op::OpPtrAccessChain:
@@ -317,12 +321,12 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
317321
ChangeResultStorageClass(inst);
318322
{
319323
std::vector<spvtools::opt::Instruction*> users;
320-
get_def_use_mgr()->ForEachUser(inst, [&users](spvtools::opt::Instruction* user) {
324+
get_def_use_mgr()->ForEachUser(&inst, [&users](spvtools::opt::Instruction* user) {
321325
users.push_back(user);
322326
});
323327
for (spvtools::opt::Instruction* user : users)
324328
{
325-
PropagateStorageClass(user, seen);
329+
PropagateStorageClass(*user, seen);
326330
}
327331
}
328332
return true;
@@ -348,16 +352,16 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
348352
default:
349353
// Unexpected pointer-producing instruction. This may indicate
350354
// a new SPIR-V extension or pattern not yet handled.
351-
UNEXPECTED("Unexpected instruction with pointer result type: opcode ", static_cast<uint32_t>(inst->opcode()));
355+
UNEXPECTED("Unexpected instruction with pointer result type: opcode ", static_cast<uint32_t>(inst.opcode()));
352356
return false;
353357
}
354358
}
355359

356360
// Changes the result type of an instruction to use the new storage class.
357-
void ChangeResultStorageClass(spvtools::opt::Instruction* inst)
361+
void ChangeResultStorageClass(spvtools::opt::Instruction& inst) const
358362
{
359363
spvtools::opt::analysis::TypeManager* type_mgr = context()->get_type_mgr();
360-
spvtools::opt::Instruction* result_type_inst = get_def_use_mgr()->GetDef(inst->type_id());
364+
spvtools::opt::Instruction* result_type_inst = get_def_use_mgr()->GetDef(inst.type_id());
361365

362366
if (result_type_inst->opcode() != spv::Op::OpTypePointer)
363367
{
@@ -368,31 +372,31 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
368372
uint32_t new_result_type_id =
369373
type_mgr->FindPointerToType(pointee_type_id, spv::StorageClass::PushConstant);
370374

371-
inst->SetResultType(new_result_type_id);
372-
context()->UpdateDefUse(inst);
375+
inst.SetResultType(new_result_type_id);
376+
context()->UpdateDefUse(&inst);
373377
}
374378

375379
// Checks if the instruction result type is a pointer.
376-
bool IsPointerResultType(spvtools::opt::Instruction* inst)
380+
bool IsPointerResultType(const spvtools::opt::Instruction& inst) const
377381
{
378-
if (inst->type_id() == 0)
382+
if (inst.type_id() == 0)
379383
{
380384
return false;
381385
}
382386

383-
spvtools::opt::Instruction* type_def = get_def_use_mgr()->GetDef(inst->type_id());
387+
spvtools::opt::Instruction* type_def = get_def_use_mgr()->GetDef(inst.type_id());
384388
return type_def != nullptr && type_def->opcode() == spv::Op::OpTypePointer;
385389
}
386390

387391
// Checks if the instruction result type is a pointer to the specified storage class.
388-
bool IsPointerToStorageClass(spvtools::opt::Instruction* inst, spv::StorageClass storage_class)
392+
bool IsPointerToStorageClass(const spvtools::opt::Instruction& inst, spv::StorageClass storage_class) const
389393
{
390-
if (inst->type_id() == 0)
394+
if (inst.type_id() == 0)
391395
{
392396
return false;
393397
}
394398

395-
spvtools::opt::Instruction* type_def = get_def_use_mgr()->GetDef(inst->type_id());
399+
spvtools::opt::Instruction* type_def = get_def_use_mgr()->GetDef(inst.type_id());
396400
if (type_def == nullptr || type_def->opcode() != spv::Op::OpTypePointer)
397401
{
398402
return false;
@@ -404,7 +408,7 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
404408
}
405409

406410
// Checks if a type has the Block decoration, which identifies it as a UBO struct type.
407-
bool HasBlockDecoration(uint32_t type_id)
411+
bool HasBlockDecoration(uint32_t type_id) const
408412
{
409413
bool has_block = false;
410414
get_decoration_mgr()->ForEachDecoration(
@@ -426,7 +430,7 @@ std::vector<uint32_t> ConvertUBOToPushConstants(
426430
{
427431
spv_target_env TargetEnv = SPIRVToolsInternal::SpvTargetEnvFromSPIRV(SPIRV);
428432

429-
spvtools::Optimizer optimizer(TargetEnv);
433+
spvtools::Optimizer optimizer{TargetEnv};
430434

431435
optimizer.SetMessageConsumer(SPIRVToolsInternal::SpvOptimizerMessageConsumer);
432436

0 commit comments

Comments
 (0)