Skip to content

Commit a364c41

Browse files
authored
We want to use extension SPV_KHR_variable_pointers with vulkan memory (KhronosGroup#6121)
model, but shader compiler crashes. The typical case is passing the buffer to a function and use it. In function, we need to load the real buffer from variable pointers, it's okay. But when we run pass `upgrade-memory-model`, we missed to handle the `OpLoad` with variable pointers, which resulted in missing 1 dimension to calculate the access element type. `OpStore` also has the same problem. Moreover, `OpLoad` and `OpStore` shouldn't be marked as `NonPrivatePointer` if the target pointer is storage class Function or Private.
1 parent 108b19e commit a364c41

File tree

2 files changed

+429
-2
lines changed

2 files changed

+429
-2
lines changed

source/opt/upgrade_memory_model.cpp

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,38 @@ void UpgradeMemoryModel::UpgradeMemoryAndImages() {
160160
}
161161

162162
switch (inst->opcode()) {
163-
case spv::Op::OpLoad:
163+
case spv::Op::OpLoad: {
164+
Instruction* src_pointer = context()->get_def_use_mgr()->GetDef(
165+
inst->GetSingleWordInOperand(0u));
166+
analysis::Type* src_type =
167+
context()->get_type_mgr()->GetType(src_pointer->type_id());
168+
auto storage_class = src_type->AsPointer()->storage_class();
169+
if (storage_class == spv::StorageClass::Function ||
170+
storage_class == spv::StorageClass::Private) {
171+
// If the buffer from function variable or private variable, flag
172+
// NonPrivatePointer is unnecessary.
173+
is_coherent = false;
174+
}
164175
UpgradeFlags(inst, 1u, is_coherent, is_volatile, kVisibility,
165176
kMemory);
166177
break;
167-
case spv::Op::OpStore:
178+
}
179+
case spv::Op::OpStore: {
180+
Instruction* src_pointer = context()->get_def_use_mgr()->GetDef(
181+
inst->GetSingleWordInOperand(0u));
182+
analysis::Type* src_type =
183+
context()->get_type_mgr()->GetType(src_pointer->type_id());
184+
auto storage_class = src_type->AsPointer()->storage_class();
185+
if (storage_class == spv::StorageClass::Function ||
186+
storage_class == spv::StorageClass::Private) {
187+
// If the buffer from function variable or private variable, flag
188+
// NonPrivatePointer is unnecessary.
189+
is_coherent = false;
190+
}
168191
UpgradeFlags(inst, 2u, is_coherent, is_volatile, kAvailability,
169192
kMemory);
170193
break;
194+
}
171195
case spv::Op::OpCopyMemory:
172196
case spv::Op::OpCopyMemorySized:
173197
start_operand = inst->opcode() == spv::Op::OpCopyMemory ? 2u : 3u;
@@ -366,6 +390,21 @@ std::pair<bool, bool> UpgradeMemoryModel::TraceInstruction(
366390
indices.push_back(inst->GetSingleWordInOperand(i));
367391
}
368392
break;
393+
case spv::Op::OpLoad:
394+
if (context()->get_type_mgr()->GetType(inst->type_id())->AsPointer()) {
395+
analysis::Integer int_ty(32, false);
396+
uint32_t int_id =
397+
context()->get_type_mgr()->GetTypeInstruction(&int_ty);
398+
const analysis::Constant* constant =
399+
context()->get_constant_mgr()->GetConstant(
400+
context()->get_type_mgr()->GetType(int_id), {0u});
401+
uint32_t constant_id = context()
402+
->get_constant_mgr()
403+
->GetDefiningInstruction(constant)
404+
->result_id();
405+
406+
indices.push_back(constant_id);
407+
}
369408
default:
370409
break;
371410
}

0 commit comments

Comments
 (0)