opt: Fold OpCompositeExtract feeding from OpCopyLogical or OpLoad#6614
opt: Fold OpCompositeExtract feeding from OpCopyLogical or OpLoad#6614s-perron wants to merge 2 commits intoKhronosGroup:mainfrom
Conversation
- CopyLogicalFeedingExtract: Hoist OpCompositeExtract before OpCopyLogical. If the input to an OpCompositeExtract is an OpCopyLogical, we can extract from the original composite and then copy the result. - LoadFeedingExtract: Change OpLoad + OpCompositeExtract to OpAccessChain + OpLoad. If the input to an OpCompositeExtract is an OpLoad, we can load the specific element instead of the entire composite. This is restricted to non-Function/Private storage classes to avoid interfering with local-access-chain-convert. Updated fold_test.cpp with 8 new test cases and updated existing tests to use SPV_ENV_UNIVERSAL_1_5. Fixes KhronosGroup#6611
|
the only catch is that you don't want to replace every I'm worried about a temp load geting used a lot getting replaced by dozens of individual redundand loads, so it would only need to consider Is this vulnerable to what I'm imagening here? |
| if (cinst->opcode() != spv::Op::OpLoad) { | ||
| return false; | ||
| } | ||
|
|
||
| Instruction* composite_type_inst = def_use_mgr->GetDef(cinst->type_id()); | ||
| if (composite_type_inst->opcode() != spv::Op::OpTypeStruct && | ||
| composite_type_inst->opcode() != spv::Op::OpTypeArray) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check the memory operands. We only support None. | ||
| if (cinst->NumInOperands() > 1) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
If I understand correctly cinst is the Instruction that produces the Operand of OpCompositeExtract and you want that to be OpLoad
The problem is that you then refuse to process an OpLoad with more than 1 argument, which would be an OpLoad with Aligned for a BDA
So this optimization pass will work on UBO and SSBO but not BDA, correct ?
There was a problem hiding this comment.
Good point. I'll try to update this.
There was a problem hiding this comment.
@devshgraphicsprogramming Let me know how this works.
- Adds `Type::GetByteOffset` to correctly compute byte offsets into
composite types, handling structs, arrays, matrices, and vectors.
- Modifies `LoadFeedingExtract` folding rule to:
- Bail out on `Volatile` loads.
- Dynamically recalculate `Aligned` loads by leveraging the
computed byte offset and the power-of-two alignment property.
- Transparently preserve and propagate all other memory operands
like `MakePointerVisible` or `Nontemporal` to the new `OpLoad`.
- Adds unit tests for the new `Type` method and updates folding
tests to ensure correct preservation of the memory operands.
3 participants
If the input to an OpCompositeExtract is an OpCopyLogical, we can
extract from the original composite and then copy the result.
If the input to an OpCompositeExtract is an OpLoad, we can load the
specific element instead of the entire composite. This is restricted
to non-Function/Private storage classes to avoid interfering with
local-access-chain-convert.
Updated fold_test.cpp with 8 new test cases and updated existing tests
to use SPV_ENV_UNIVERSAL_1_5.
Fixes #6611