-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[X86][SelectionDAG] Handle the case for gather where index is SHL #139703
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-backend-x86 Author: Rohit Aggarwal (rohitaggarwal007) ChangesFix the Gather's Index for SHL Opcode in which shift amount is 4 or greater. It is in the continuity of #137813 @RKSimon Please review this. Full diff: https://github.com/llvm/llvm-project/pull/139703.diff 1 Files Affected:
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index ac4fb157a6026..4bb23ced2bc42 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -56710,12 +56710,23 @@ static SDValue combineGatherScatter(SDNode *N, SelectionDAG &DAG,
if (DCI.isBeforeLegalize()) {
unsigned IndexWidth = Index.getScalarValueSizeInBits();
-
+ // If the index is a left shift, \ComputeNumSignBits we are recomputing
+ // the number of sign bits from the shifted value. We are trying to enable
+ // the optimization in which we can shrink indices if they are larger than
+ // 32-bits. Using the existing fold techniques implemented below.
+ unsigned ComputeNumSignBits = DAG.ComputeNumSignBits(Index);
+ if (Index.getOpcode() == ISD::SHL) {
+ if (auto MinShAmt = DAG.getValidMinimumShiftAmount(Index)) {
+ if (DAG.ComputeNumSignBits(Index.getOperand(0)) > 1) {
+ ComputeNumSignBits += *MinShAmt;
+ }
+ }
+ }
// Shrink indices if they are larger than 32-bits.
// Only do this before legalize types since v2i64 could become v2i32.
// FIXME: We could check that the type is legal if we're after legalize
// types, but then we would need to construct test cases where that happens.
- if (IndexWidth > 32 && DAG.ComputeNumSignBits(Index) > (IndexWidth - 32)) {
+ if (IndexWidth > 32 && ComputeNumSignBits > (IndexWidth - 32)) {
EVT NewVT = IndexVT.changeVectorElementType(MVT::i32);
// FIXME: We could support more than just constant fold, but we need to
|
@rohitaggarwal007 please can you rebase against trunk? |
2b79bbe
to
7038d50
Compare
…ject into gatherSHLIndex
✅ With the latest revision this PR passed the C/C++ code formatter. |
Done |
; X64-KNL-NEXT: vgatherqps (%rdi,%zmm0), %ymm1 {%k1} | ||
; X64-KNL-NEXT: vinsertf64x4 $1, %ymm3, %zmm1, %zmm0 | ||
; X64-KNL-NEXT: vpslld $4, (%rsi), %zmm0 | ||
; X64-KNL-NEXT: vgatherdps (%rdi,%zmm0), %zmm1 {%k1} |
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 still seems wrong - afaict you are doing this:
define i64 @src(i32 noundef %x) {
#0:
%and = and i32 noundef %x, 536870911
%zext = zext i32 %and to i64
%hi = shl i64 %zext, 4
ret i64 %hi
}
=>
define i64 @tgt(i32 noundef %x) {
#0:
%shl = shl i32 noundef %x, 4
%ext = sext i32 %shl to i64
ret i64 %ext
}
Transformation doesn't verify!
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.
So the problem is that we can overflow the value on left shift, right?
Fix the Gather's Index for SHL Opcode in which shift amount is 4 or greater.
It is in the continuity of #137813
@RKSimon Please review this.