-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[X86][SelectionDAG] Fix the Gather's base and index by modifying the Scale value #137813
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
Changes from 27 commits
741acb0
9d395b1
0625ae4
84d9a5f
76d9167
b73051c
a43aa1d
03b3daf
0e8856f
7eb7663
4ed4a4d
0c4eb0f
dc1b6d6
34df281
5aaa2ab
8ce9360
bfbbe9f
4bb9f5b
fb131f8
47bf70b
716943e
b9e571a
f0a175a
4b57e4a
8995df9
5a12c7d
d2f1352
ed72aa3
1644def
7ca7d48
94f16e6
cfcf542
7ab9924
62fa5ad
ad2e419
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56578,13 +56578,49 @@ static SDValue combineGatherScatter(SDNode *N, SelectionDAG &DAG, | |
const TargetLowering &TLI = DAG.getTargetLoweringInfo(); | ||
|
||
if (DCI.isBeforeLegalize()) { | ||
// Attempt to move shifted index into the address scale, allows further | ||
// index truncation below. | ||
if (Index.getOpcode() == ISD::SHL && isa<ConstantSDNode>(Scale)) { | ||
unsigned BitWidth = Index.getScalarValueSizeInBits(); | ||
unsigned MaskBits = BitWidth - Log2_32(Scale->getAsZExtVal()); | ||
APInt DemandedBits = APInt::getLowBitsSet(BitWidth, MaskBits); | ||
if (TLI.SimplifyDemandedBits(Index, DemandedBits, DCI)) { | ||
if (N->getOpcode() != ISD::DELETED_NODE) | ||
DCI.AddToWorklist(N); | ||
return SDValue(N, 0); | ||
} | ||
uint64_t ScaleAmt = Scale->getAsZExtVal(); | ||
if (auto MinShAmt = DAG.getValidMinimumShiftAmount(Index)) { | ||
if (*MinShAmt >= 1 && (*MinShAmt + Log2_64(ScaleAmt)) < 4 && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. merge Log2_64 call with the existing Log2_32 call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
DAG.ComputeNumSignBits(Index.getOperand(0)) > 1) { | ||
SDValue ShAmt = Index.getOperand(1); | ||
SDValue NewShAmt = | ||
DAG.getNode(ISD::SUB, DL, ShAmt.getValueType(), ShAmt, | ||
DAG.getConstant(1, DL, ShAmt.getValueType())); | ||
SDValue NewIndex = DAG.getNode(ISD::SHL, DL, Index.getValueType(), | ||
Index.getOperand(0), NewShAmt); | ||
SDValue NewScale = | ||
DAG.getConstant(ScaleAmt * 2, DL, Scale.getValueType()); | ||
return rebuildGatherScatter(GorS, NewIndex, Base, NewScale, DAG); | ||
} | ||
} | ||
} | ||
unsigned IndexWidth = Index.getScalarValueSizeInBits(); | ||
|
||
// 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)) { | ||
unsigned ComputeNumSignBits = DAG.ComputeNumSignBits(Index); | ||
if (Index.getOpcode() == ISD::SHL) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All this code needs moving inside the IndexWidth > 32 check - needs commenting as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
if (auto MinShAmt = DAG.getValidMinimumShiftAmount(Index)) { | ||
if (DAG.ComputeNumSignBits(Index.getOperand(0)) > 1) { | ||
ComputeNumSignBits += *MinShAmt; | ||
} | ||
} | ||
} | ||
|
||
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 | ||
|
@@ -56604,11 +56640,25 @@ static SDValue combineGatherScatter(SDNode *N, SelectionDAG &DAG, | |
Index = DAG.getNode(ISD::TRUNCATE, DL, NewVT, Index); | ||
return rebuildGatherScatter(GorS, Index, Base, Scale, DAG); | ||
} | ||
|
||
// Shrink if we remove an illegal type. | ||
if (!TLI.isTypeLegal(Index.getValueType()) && TLI.isTypeLegal(NewVT)) { | ||
Index = DAG.getNode(ISD::TRUNCATE, DL, NewVT, Index); | ||
return rebuildGatherScatter(GorS, Index, Base, Scale, DAG); | ||
} | ||
} | ||
} | ||
|
||
EVT PtrVT = TLI.getPointerTy(DAG.getDataLayout()); | ||
|
||
// if (Index.getOpcode() == ISD::SHL) { | ||
// unsigned BitWidth = Index.getScalarValueSizeInBits(); | ||
// unsigned MaskBits = BitWidth - Log2_32(Scale->getAsZExtVal()); | ||
// APInt DemandedBits = APInt::getLowBitsSet(BitWidth, MaskBits); | ||
// if (TLI.SimplifyDemandedBits(Index, DemandedBits, DCI)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hoist the equivalent IndexWidth above this and remove BitWidth There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// return SDValue(N, 0); | ||
// } | ||
// } | ||
RKSimon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Try to move splat adders from the index operand to the base | ||
// pointer operand. Taking care to multiply by the scale. We can only do | ||
// this when index element type is the same as the pointer type. | ||
|
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.
move this up and reuse in Log2_32 check - we should probably assert that ScaleAmt is a power of 2 as well
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.
Done