Skip to content

[X86] combineShiftRightLogical - fold srl(vecreduce_umax(x),bw-1) as MOVMSK signbit reduction #210281

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
RKSimon merged 4 commits into from
Jul 20, 2026
Merged

[X86] combineShiftRightLogical - fold srl(vecreduce_umax(x),bw-1) as MOVMSK signbit reduction #210281

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
271b338
[X86] Add test coverage for #209714
RKSimon Jul 17, 2026
a3c7966
[X86] combineShiftRightLogical - fold srl(vecreduce_umax(x),bw-1) as …
RKSimon Jul 17, 2026
9d94d0b
Add 512-bit test coverage (and removed some unused NOT copy+pasta)
RKSimon Jul 20, 2026
a7cd7ac
Add 512-bit vector handling
RKSimon Jul 20, 2026
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
27 changes: 27 additions & 0 deletions llvm/lib/Target/X86/X86ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50910,6 +50910,33 @@ static SDValue combineShiftRightLogical(SDNode *N, SelectionDAG &DAG,
}
}

// VectorCombine may have folded:
// icmp_eq(vecreduce_or(splatsign(x)),0) --> icmp_sgt(vecreduce_umax(x),-1)
// which DAG folds to: srl(vecreduce_umax(x),bw-1).
// This attempts to reconstruct the signbit reduction.
if (sd_match(N1, m_SpecificInt(EltSizeInBits - 1))) {
SDValue X = N0;
ISD::CondCode CC = ISD::SETNE;
if (sd_match(N0, m_Not(m_Value(X))))
CC = ISD::SETEQ;
if (X.getOpcode() == ISD::VECREDUCE_UMAX) {
SDValue V = X.getOperand(0);
EVT VecVT = V.getValueType();
if (DAG.getTargetLoweringInfo().isTypeLegal(VecVT) &&
(VecVT.is128BitVector() || VecVT.is256BitVector())) {

@phoebewang phoebewang Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why exclude 512-bit?

@RKSimon RKSimon Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll add test coverage to check how bad 512-bit cases look like

if (VecVT == MVT::v16i16) {
auto [Lo, Hi] = DAG.SplitVector(V, DL);
V = DAG.getNode(X86ISD::PACKSS, DL, MVT::v16i8, Lo, Hi);
} else if (VecVT == MVT::v8i16) {
V = DAG.getNode(X86ISD::PACKSS, DL, MVT::v16i8, V, V);
}
V = getPMOVMSKB(DL, V, DAG, Subtarget);

@phoebewang phoebewang Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC, getPMOVMSKB only handles vXi8 type, how vXi32/vXi64 get lowered by this?

@RKSimon RKSimon Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Its a badly named function - it was originally added to handle v32i8/v64i8 splitting, but has been used for more general MOVMSK lowering for some time. If you want I can rename it (and add some much needed asserts), but was trying to minimise churn for a release patch.

@phoebewang phoebewang Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. I agree it's good to do it in a follow up.

V = DAG.getSetCC(DL, MVT::i8, V, DAG.getConstant(0, DL, MVT::i32), CC);
return DAG.getZExtOrTrunc(V, DL, VT);
}
}
}

// Only do this on the last DAG combine as it can interfere with other
// combines.
if (!DCI.isAfterLegalizeDAG())
Expand Down
Loading
Loading