Skip to content

[DirectX] Fix GEP flattening with 0-indexed GEPs on global variables #149211

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.

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 1 commit into from
Jul 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions llvm/lib/Target/DirectX/DXILFlattenArrays.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,16 @@ bool DXILFlattenArraysVisitor::visitGetElementPtrInst(GetElementPtrInst &GEP) {
Info.RootFlattenedArrayType, Info.RootPointerOperand,
{ZeroIndex, FlattenedIndex}, GEP.getName(), GEP.getNoWrapFlags());

// If the pointer operand is a global variable and all indices are 0,
// IRBuilder::CreateGEP will return the global variable instead of creating
// a GEP instruction or GEP ConstantExpr. In this case we have to create and
// insert our own GEP instruction.
if (!isa<GEPOperator>(NewGEP))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there something preventing us from just doing this directly the first time?

Copy link
Contributor Author

@Icohedron Icohedron Jul 17, 2025

Choose a reason for hiding this comment

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

If I wanted to always create a GEP Instruction, then sure we don't need this if statement and can just use GetElementPtrInst::Create without using the IRBuilder.
But I am keeping the ability to let IRBuilder::CreateGEP create GEP ConstantExprs as well.

NewGEP = GetElementPtrInst::Create(
Info.RootFlattenedArrayType, Info.RootPointerOperand,
{ZeroIndex, FlattenedIndex}, GEP.getNoWrapFlags(), GEP.getName(),
Builder.GetInsertPoint());

// Replace the current GEP with the new GEP. Store GEPInfo into the map
// for later use in case this GEP was not the end of the chain
GEPChainInfoMap.insert({cast<GEPOperator>(NewGEP), std::move(Info)});
Expand Down
22 changes: 22 additions & 0 deletions llvm/test/CodeGen/DirectX/flatten-array.ll
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,28 @@ define void @two_index_gep_const() {
ret void
}

define void @zero_index_global() {
; CHECK-LABEL: define void @zero_index_global(
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds nuw [4 x float], ptr addrspace(3) @g.1dim, i32 0, i32 0
; CHECK-NEXT: load float, ptr addrspace(3) [[GEP]], align 4
; CHECK-NEXT: ret void
%1 = getelementptr inbounds nuw [2 x [2 x float]], ptr addrspace(3) @g, i32 0, i32 0, i32 0
%2 = load float, ptr addrspace(3) %1, align 4
ret void
}

; Note: A ConstantExpr GEP with all 0 indices is equivalent to the pointer
; operand of the GEP. Therefore the visitLoadInst will not see the pointer operand
; as a ConstantExpr GEP and will not create a GEP instruction to be visited.
; The later dxil-legalize pass will insert a GEP in this instance.
define void @zero_index_global_const() {
; CHECK-LABEL: define void @zero_index_global_const(
; CHECK-NEXT: load float, ptr addrspace(3) @g.1dim, align 4
; CHECK-NEXT: ret void
%1 = load float, ptr addrspace(3) getelementptr inbounds nuw ([2 x [2 x float]], ptr addrspace(3) @g, i32 0, i32 0, i32 0), align 4
ret void
}

define void @gep_4d_index_test() {
; CHECK-LABEL: gep_4d_index_test
; CHECK: [[a:%.*]] = alloca [16 x i32], align 4
Expand Down
Loading