Skip to content

Commit fbc0d53

Browse files
committed
[RISCV] Don't run combineBinOp_VLToVWBinOp_VL until after legalize types. NFCI
I noticed this from a discrepancy in fillUpExtensionSupport between how we apparently need to check for legal types for ISD::{ZERO,SIGN}_EXTEND, but we don't need to for RISCVISD::V{Z,S}EXT_VL. Prior to llvm#72340, combineBinOp_VLToVWBinOp_VL only ran after type legalization because it only operated on _VL nodes. _VL nodes are only emitted during op legalization, which takes place **after** type legalization, which is presumably why the existing code didn't need to check for legal types. After llvm#72340 we now handle generic ops like ISD::ADD that exist before op legalization and thus **before** type legalization. This meant that we needed to add extra checks that the narrow type was legal in llvm#76785. I think the easiest thing to do here is to just maintain the invariant that the types are legal and only run the combine after type legalization.
1 parent bec7ad9 commit fbc0d53

File tree

1 file changed

+10
-14
lines changed

1 file changed

+10
-14
lines changed

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13652,10 +13652,6 @@ struct NodeExtensionHelper {
1365213652
unsigned ScalarBits = VT.getScalarSizeInBits();
1365313653
unsigned NarrowScalarBits = NarrowVT.getScalarSizeInBits();
1365413654

13655-
// Ensure the narrowing element type is legal
13656-
if (!Subtarget.getTargetLowering()->isTypeLegal(NarrowElt.getValueType()))
13657-
break;
13658-
1365913655
// Ensure the extension's semantic is equivalent to rvv vzext or vsext.
1366013656
if (ScalarBits != NarrowScalarBits * 2)
1366113657
break;
@@ -13727,14 +13723,11 @@ struct NodeExtensionHelper {
1372713723
}
1372813724

1372913725
/// Check if \p Root supports any extension folding combines.
13730-
static bool isSupportedRoot(const SDNode *Root, const SelectionDAG &DAG) {
13731-
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
13726+
static bool isSupportedRoot(const SDNode *Root) {
1373213727
switch (Root->getOpcode()) {
1373313728
case ISD::ADD:
1373413729
case ISD::SUB:
1373513730
case ISD::MUL: {
13736-
if (!TLI.isTypeLegal(Root->getValueType(0)))
13737-
return false;
1373813731
return Root->getValueType(0).isScalableVector();
1373913732
}
1374013733
// Vector Widening Integer Add/Sub/Mul Instructions
@@ -13751,7 +13744,7 @@ struct NodeExtensionHelper {
1375113744
case RISCVISD::FMUL_VL:
1375213745
case RISCVISD::VFWADD_W_VL:
1375313746
case RISCVISD::VFWSUB_W_VL:
13754-
return TLI.isTypeLegal(Root->getValueType(0));
13747+
return true;
1375513748
default:
1375613749
return false;
1375713750
}
@@ -13760,9 +13753,10 @@ struct NodeExtensionHelper {
1376013753
/// Build a NodeExtensionHelper for \p Root.getOperand(\p OperandIdx).
1376113754
NodeExtensionHelper(SDNode *Root, unsigned OperandIdx, SelectionDAG &DAG,
1376213755
const RISCVSubtarget &Subtarget) {
13763-
assert(isSupportedRoot(Root, DAG) && "Trying to build an helper with an "
13764-
"unsupported root");
13756+
assert(isSupportedRoot(Root) && "Trying to build an helper with an "
13757+
"unsupported root");
1376513758
assert(OperandIdx < 2 && "Requesting something else than LHS or RHS");
13759+
assert(DAG.getTargetLoweringInfo().isTypeLegal(Root->getValueType(0)));
1376613760
OrigOperand = Root->getOperand(OperandIdx);
1376713761

1376813762
unsigned Opc = Root->getOpcode();
@@ -13812,7 +13806,7 @@ struct NodeExtensionHelper {
1381213806
static std::pair<SDValue, SDValue>
1381313807
getMaskAndVL(const SDNode *Root, SelectionDAG &DAG,
1381413808
const RISCVSubtarget &Subtarget) {
13815-
assert(isSupportedRoot(Root, DAG) && "Unexpected root");
13809+
assert(isSupportedRoot(Root) && "Unexpected root");
1381613810
switch (Root->getOpcode()) {
1381713811
case ISD::ADD:
1381813812
case ISD::SUB:
@@ -14112,8 +14106,10 @@ static SDValue combineBinOp_VLToVWBinOp_VL(SDNode *N,
1411214106
TargetLowering::DAGCombinerInfo &DCI,
1411314107
const RISCVSubtarget &Subtarget) {
1411414108
SelectionDAG &DAG = DCI.DAG;
14109+
if (DCI.isBeforeLegalize())
14110+
return SDValue();
1411514111

14116-
if (!NodeExtensionHelper::isSupportedRoot(N, DAG))
14112+
if (!NodeExtensionHelper::isSupportedRoot(N))
1411714113
return SDValue();
1411814114

1411914115
SmallVector<SDNode *> Worklist;
@@ -14124,7 +14120,7 @@ static SDValue combineBinOp_VLToVWBinOp_VL(SDNode *N,
1412414120

1412514121
while (!Worklist.empty()) {
1412614122
SDNode *Root = Worklist.pop_back_val();
14127-
if (!NodeExtensionHelper::isSupportedRoot(Root, DAG))
14123+
if (!NodeExtensionHelper::isSupportedRoot(Root))
1412814124
return SDValue();
1412914125

1413014126
NodeExtensionHelper LHS(N, 0, DAG, Subtarget);

0 commit comments

Comments
 (0)