Skip to content

Commit

Permalink
Revert "[RISCV] Generaize reduction tree matching to all integer redu…
Browse files Browse the repository at this point in the history
…ctions (#68014)"

This reverts commit 7a0b9da and
63bbc25.

I'm seeing issues (e.g. on the GCC torture suite) where
combineBinOpOfExtractToReduceTree is called when the V extensions aren't
enabled and triggers a crash due to RISCVSubtarget::getElen asserting.

I'll aim to follow up with a minimal reproducer. Although it's pretty
obvious how to avoid this crash with some extra gating, there are a few
options as to where that should be inserted so I think it's best to
revert and agree the appropriate fix separately.
  • Loading branch information
asb committed Oct 4, 2023
1 parent 2be7c65 commit 824251c
Show file tree
Hide file tree
Showing 3 changed files with 646 additions and 867 deletions.
58 changes: 7 additions & 51 deletions llvm/lib/Target/RISCV/RISCVISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11112,31 +11112,6 @@ void RISCVTargetLowering::ReplaceNodeResults(SDNode *N,
}
}

/// Given an integer binary operator, return the generic ISD::VECREDUCE_OP
/// which corresponds to it.
static unsigned getVecReduceOpcode(unsigned Opc) {
switch (Opc) {
default:
llvm_unreachable("Unhandled binary to transfrom reduction");
case ISD::ADD:
return ISD::VECREDUCE_ADD;
case ISD::UMAX:
return ISD::VECREDUCE_UMAX;
case ISD::SMAX:
return ISD::VECREDUCE_SMAX;
case ISD::UMIN:
return ISD::VECREDUCE_UMIN;
case ISD::SMIN:
return ISD::VECREDUCE_SMIN;
case ISD::AND:
return ISD::VECREDUCE_AND;
case ISD::OR:
return ISD::VECREDUCE_OR;
case ISD::XOR:
return ISD::VECREDUCE_XOR;
}
}

/// Perform two related transforms whose purpose is to incrementally recognize
/// an explode_vector followed by scalar reduction as a vector reduction node.
/// This exists to recover from a deficiency in SLP which can't handle
Expand All @@ -11155,15 +11130,8 @@ combineBinOpOfExtractToReduceTree(SDNode *N, SelectionDAG &DAG,

const SDLoc DL(N);
const EVT VT = N->getValueType(0);

// TODO: Handle floating point here.
if (!VT.isInteger())
return SDValue();

const unsigned Opc = N->getOpcode();
const unsigned ReduceOpc = getVecReduceOpcode(Opc);
assert(Opc == ISD::getVecReduceBaseOpcode(ReduceOpc) &&
"Inconsistent mappings");
[[maybe_unused]] const unsigned Opc = N->getOpcode();
assert(Opc == ISD::ADD && "extend this to other reduction types");
const SDValue LHS = N->getOperand(0);
const SDValue RHS = N->getOperand(1);

Expand Down Expand Up @@ -11193,13 +11161,13 @@ combineBinOpOfExtractToReduceTree(SDNode *N, SelectionDAG &DAG,
EVT ReduceVT = EVT::getVectorVT(*DAG.getContext(), VT, 2);
SDValue Vec = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, ReduceVT, SrcVec,
DAG.getVectorIdxConstant(0, DL));
return DAG.getNode(ReduceOpc, DL, VT, Vec);
return DAG.getNode(ISD::VECREDUCE_ADD, DL, VT, Vec);
}

// Match (binop (reduce (extract_subvector V, 0),
// (extract_vector_elt V, sizeof(SubVec))))
// into a reduction of one more element from the original vector V.
if (LHS.getOpcode() != ReduceOpc)
if (LHS.getOpcode() != ISD::VECREDUCE_ADD)
return SDValue();

SDValue ReduceVec = LHS.getOperand(0);
Expand All @@ -11215,7 +11183,7 @@ combineBinOpOfExtractToReduceTree(SDNode *N, SelectionDAG &DAG,
EVT ReduceVT = EVT::getVectorVT(*DAG.getContext(), VT, Idx + 1);
SDValue Vec = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, ReduceVT, SrcVec,
DAG.getVectorIdxConstant(0, DL));
return DAG.getNode(ReduceOpc, DL, VT, Vec);
return DAG.getNode(ISD::VECREDUCE_ADD, DL, VT, Vec);
}
}

Expand Down Expand Up @@ -11723,8 +11691,6 @@ static SDValue performANDCombine(SDNode *N,

if (SDValue V = combineBinOpToReduce(N, DAG, Subtarget))
return V;
if (SDValue V = combineBinOpOfExtractToReduceTree(N, DAG, Subtarget))
return V;

if (DCI.isAfterLegalizeDAG())
if (SDValue V = combineDeMorganOfBoolean(N, DAG))
Expand Down Expand Up @@ -11777,8 +11743,6 @@ static SDValue performORCombine(SDNode *N, TargetLowering::DAGCombinerInfo &DCI,

if (SDValue V = combineBinOpToReduce(N, DAG, Subtarget))
return V;
if (SDValue V = combineBinOpOfExtractToReduceTree(N, DAG, Subtarget))
return V;

if (DCI.isAfterLegalizeDAG())
if (SDValue V = combineDeMorganOfBoolean(N, DAG))
Expand Down Expand Up @@ -11830,9 +11794,6 @@ static SDValue performXORCombine(SDNode *N, SelectionDAG &DAG,

if (SDValue V = combineBinOpToReduce(N, DAG, Subtarget))
return V;
if (SDValue V = combineBinOpOfExtractToReduceTree(N, DAG, Subtarget))
return V;

// fold (xor (select cond, 0, y), x) ->
// (select cond, x, (xor x, y))
return combineSelectAndUseCommutative(N, DAG, /*AllOnes*/ false, Subtarget);
Expand Down Expand Up @@ -14038,13 +13999,8 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
case ISD::SMAX:
case ISD::SMIN:
case ISD::FMAXNUM:
case ISD::FMINNUM: {
if (SDValue V = combineBinOpToReduce(N, DAG, Subtarget))
return V;
if (SDValue V = combineBinOpOfExtractToReduceTree(N, DAG, Subtarget))
return V;
return SDValue();
}
case ISD::FMINNUM:
return combineBinOpToReduce(N, DAG, Subtarget);
case ISD::SETCC:
return performSETCCCombine(N, DAG, Subtarget);
case ISD::SIGN_EXTEND_INREG:
Expand Down

0 comments on commit 824251c

Please sign in to comment.