Skip to content

[DirectX] Do not flatten GEP chains for unsupported types #150484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 24, 2025
Merged

[DirectX] Do not flatten GEP chains for unsupported types #150484

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ec8fc4
Do not flatten GEP chains for unsupported types
Icohedron Jul 24, 2025
3faecb6
Merge branch 'main' into flatten-structs
Icohedron Jul 24, 2025
733ae8e
Add a test to ensure there are no errors
Icohedron Jul 24, 2025
9250cad
Clarify comment about not processing structs or GEPs for structs
Icohedron Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions llvm/lib/Target/DirectX/DXILFlattenArrays.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,13 @@ bool DXILFlattenArraysVisitor::visitGetElementPtrInst(GetElementPtrInst &GEP) {
// merge the byte offsets. Otherwise, this GEP is itself the root of a GEP
// chain and we need to deterine the root array type
if (auto *PtrOpGEP = dyn_cast<GEPOperator>(PtrOperand)) {
assert(GEPChainInfoMap.contains(PtrOpGEP) &&
"Expected parent GEP to be visited before this GEP");

// If the parent GEP was not processed, then we do not want to process its
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment makes sense as does the fix, Was wondering should we have something more explict about type checking for structs and bailing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I think the comment on 296 is a good spot to indicate that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could throw an error when there is an unsupported type, but would the DXILFlattenArrays pass really be the place to throw such errors?

Line 296 and below already has

    // If the root type is not an array, we don't need to do any flattening
    if (!isa<ArrayType>(RootTy))
      return false;

to indicate that only arrays are applicable to this transformation.
The name of the pass "DXIL Flatten Arrays" to me also implies it only applies to arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be addressed in a follow-up PR if needed.

// descendants. This can happen if the GEP chain is for an unsupported type
// such as structs -- we do not flatten structs.
if (!GEPChainInfoMap.contains(PtrOpGEP))
return false;

GEPInfo &PGEPInfo = GEPChainInfoMap[PtrOpGEP];
Info.RootFlattenedArrayType = PGEPInfo.RootFlattenedArrayType;
Info.RootPointerOperand = PGEPInfo.RootPointerOperand;
Expand Down
2 changes: 2 additions & 0 deletions llvm/test/CodeGen/DirectX/issue-145408-gep-struct-fix.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ define void @test_no_transform_of_struct() {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[OUTPUTSIZESLOCAL_I:%.*]] = alloca [[STRUCT_RAWSTRUCT8D:%.*]], align 4
; CHECK-NEXT: [[ARRAYINIT_ELEMENT13_I76:%.*]] = getelementptr inbounds nuw [1 x %struct.RawStruct8D], ptr [[OUTPUTSIZESLOCAL_I]], i32 0, i32 0
; CHECK-NEXT: [[ARRAYINIT_ELEMENT13_I76_I1:%.*]] = getelementptr inbounds nuw [8 x i32], ptr [[ARRAYINIT_ELEMENT13_I76]], i32 0, i32 1
; CHECK-NEXT: ret void
;
entry:
%outputSizesLocal.i = alloca %struct.RawStruct8D, align 4
%arrayinit.element13.i76 = getelementptr inbounds nuw [1 x %struct.RawStruct8D], ptr %outputSizesLocal.i, i32 0, i32 0
%arrayinit.element13.i76.i1 = getelementptr inbounds nuw [8 x i32], ptr %arrayinit.element13.i76, i32 0, i32 1
ret void
}
Loading