Skip to content

Commit 927abee

Browse files
committed
Fixed an issue that new pointer type created by ConvertUBOToPushConstantsPass violates SPIR-V validation rules.
ConvertUBOToPushConstants should return empty SPRIV when block not found.
1 parent 973c29e commit 927abee

File tree

1 file changed

+32
-9
lines changed

1 file changed

+32
-9
lines changed

Graphics/ShaderTools/src/ConvertUBOToPushConstant.cpp

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
9696
if (candidate_ids.empty())
9797
{
9898
// Block name not found
99-
return Status::SuccessWithoutChange;
99+
return Status::Failure;
100100
}
101101

102102
// Try each candidate ID to find a UniformBuffer
@@ -191,7 +191,7 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
191191
if (target_var == nullptr)
192192
{
193193
// No UniformBuffer found with the given block name
194-
return Status::SuccessWithoutChange;
194+
return Status::Failure;
195195
}
196196

197197
uint32_t target_var_id = target_var->result_id();
@@ -217,11 +217,23 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
217217
return Status::Failure;
218218
}
219219

220-
// Note: We don't need to manually reorder the pointer type instruction.
221-
// SPIR-V does not require types to appear in any specific relative order,
222-
// as long as they are valid type declarations. FindPointerToType handles
223-
// type creation correctly, and the SPIRV-Tools framework manages instruction
224-
// ordering appropriately.
220+
// IMPORTANT: FindPointerToType() may create a new type instruction at the end of
221+
// the types_values section. However, SPIR-V requires all IDs to be defined before
222+
// use (SSA form). If the new pointer type was just created, it will appear AFTER
223+
// the OpVariable that uses it, which violates SPIR-V validation rules.
224+
//
225+
// We need to move the newly created pointer type instruction to appear BEFORE
226+
// the OpVariable instruction that will reference it.
227+
spvtools::opt::Instruction* new_ptr_type_inst = get_def_use_mgr()->GetDef(new_ptr_type_id);
228+
if (new_ptr_type_inst != nullptr && new_ptr_type_inst != ptr_type_inst)
229+
{
230+
// The new pointer type was created (not reused). Move it before the variable.
231+
// In SPIR-V, types appear in the types_values section before variables.
232+
// We insert the new type right after the original pointer type to maintain
233+
// proper ordering within the types section.
234+
new_ptr_type_inst->RemoveFromList();
235+
new_ptr_type_inst->InsertAfter(ptr_type_inst);
236+
}
225237

226238
// Update the variable's type to the new pointer type
227239
target_var->SetResultType(new_ptr_type_id);
@@ -372,8 +384,19 @@ class ConvertUBOToPushConstantPass : public spvtools::opt::Pass
372384
uint32_t new_result_type_id =
373385
type_mgr->FindPointerToType(pointee_type_id, spv::StorageClass::PushConstant);
374386

375-
inst.SetResultType(new_result_type_id);
376-
context()->UpdateDefUse(&inst);
387+
// If a new type was created, ensure it's properly positioned in the types section.
388+
// FindPointerToType may append new types at the end, but they need to appear
389+
// before any instructions that use them (SPIR-V SSA requirement).
390+
spvtools::opt::Instruction* new_type_inst = get_def_use_mgr()->GetDef(new_result_type_id);
391+
if (new_type_inst != nullptr && new_type_inst != result_type_inst)
392+
{
393+
// Move the new type to appear after the original type in the types section
394+
new_type_inst->RemoveFromList();
395+
new_type_inst->InsertAfter(result_type_inst);
396+
}
397+
398+
inst->SetResultType(new_result_type_id);
399+
context()->UpdateDefUse(inst);
377400
}
378401

379402
// Checks if the instruction result type is a pointer.

0 commit comments

Comments
 (0)