Skip to content

Commit

Permalink
spirv-opt: fix copy-propagate-arrays index opti on structs.
Browse files Browse the repository at this point in the history
As per SPIR-V spec:
OpAccessChain indices must be OpConstant when indexing into a structure.

This optimization tried to remove load cascade. But in some scenario
failed:

```c
cbuffer MyStruct {
    uint my_field;
};

uint main(uint index) {
    const uint my_array[1] = { my_field };
    return my_array[index]
}
```

This is valid as the struct is indexed with a constant index, and then
the array is indexed using a dynamic index.
The optimization would consider the local array to be useless and
generated a load directly into the struct.

Fixes KhronosGroup#4887

Signed-off-by: Nathan Gauër <brioche@google.com>
  • Loading branch information
Keenuts committed Aug 9, 2022
1 parent 09aa8b6 commit 5405223
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion source/opt/copy_prop_arrays.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ bool CopyPropagateArrays::CanUpdateUses(Instruction* original_ptr_inst,

return def_use_mgr->WhileEachUse(original_ptr_inst, [this, type_mgr,
const_mgr,
def_use_mgr,
type](Instruction* use,
uint32_t) {
if (IsDebugDeclareOrValue(use)) return true;
Expand All @@ -524,6 +525,11 @@ bool CopyPropagateArrays::CanUpdateUses(Instruction* original_ptr_inst,
analysis::Pointer* pointer_type = type->AsPointer();
const analysis::Type* pointee_type = pointer_type->pointee_type();

// Indexes must be an OpConstant when indexing in a struct. Cannot optimize further.
if (pointee_type->kind() == analysis::Type::kStruct) {
return false;
}

std::vector<uint32_t> access_chain;
for (uint32_t i = 1; i < use->NumInOperands(); ++i) {
const analysis::Constant* index_const =
Expand All @@ -542,7 +548,7 @@ bool CopyPropagateArrays::CanUpdateUses(Instruction* original_ptr_inst,
analysis::Pointer pointerTy(new_pointee_type,
pointer_type->storage_class());
uint32_t new_pointer_type_id =
context()->get_type_mgr()->GetTypeInstruction(&pointerTy);
type_mgr->GetTypeInstruction(&pointerTy);
if (new_pointer_type_id == 0) {
return false;
}
Expand Down

0 comments on commit 5405223

Please sign in to comment.