-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[DirectX] Support ConstExpr GEPs #148986
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
[DirectX] Support ConstExpr GEPs #148986
Conversation
// Check if the pointer operand is a ConstantExpr GEP | ||
if (auto *PtrOpGEPCE = dyn_cast<ConstantExpr>(PtrOperand); | ||
PtrOpGEPCE && PtrOpGEPCE->getOpcode() == Instruction::GetElementPtr) { | ||
if (GlobalVariable *NewGlobal = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: The pointer operand of a ConstantExpr GEP can be another ConstantExpr GEP.
Example:
llvm-project/llvm/test/CodeGen/DirectX/flatten-array.ll
Lines 136 to 142 in 66e707e
define void @global_nested_geps() { | |
; CHECK-LABEL: define void @global_nested_geps( | |
; CHECK: {{.*}} = load i32, ptr getelementptr inbounds ([24 x i32], ptr @a.1dim, i32 0, i32 6), align 4 | |
; CHECK-NEXT: ret void | |
%1 = load i32, i32* getelementptr inbounds ([4 x i32], [4 x i32]* getelementptr inbounds ([3 x [4 x i32]], [3 x [4 x i32]]* getelementptr inbounds ([2 x [3 x [4 x i32]]], [2 x [3 x [4 x i32]]]* @a, i32 0, i32 0), i32 0, i32 1), i32 0, i32 2), align 4 | |
ret void | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was afraid of that. Then we will have to make this recursive for constexprs. Which sucks that we will have to do this twice once for data scalarization and then again for flattening. I'm wondering if it wouldn't just make more sense to undo all the constExprs before these passes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it will ever naturally occur though. It may suffice to assert that the ConstantExpr GEP's pointer operand is a global variable and deal with it later if it does become an actual problem.
AFAIK there isn't a good reason to codegen a multiply-nested ConstantExpr GEP in the first place when you can codegen a single ConstantExpr GEP instead because all the indices are ConstantInts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR supercedes this one and will fix the multilple nesting case: #150082
This change does fix a bug, but I think we need to file a different ticket for this case: groupshared uint4 aTile[10][10];
[numthreads(1, 1, 1)]
void CSMain() {
uint4 aFragPacked = aTile[1][2];
} Need to rename the branch and then we don't have to call this a partial fix. |
The issue still needs to be filed, but this PR partially addresses the Data Scalarization pass failing to handle nested constantexpr GEPs.
Farzon wants to rethink this solution to make it recursive, so once the issue is filed/that solution is put up, this PR will be closed.