From 9f215be792d269e58aaaef3393ceff618d071e1e Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Tue, 22 Apr 2025 14:24:24 +0800 Subject: [PATCH 1/4] [DAGCombiner][RISCV] Add target hook to decide hoisting LogicOp with extension. This patch introduces a new target hook `isDesirableToHoistLogicOpWithExt` to allow target to decide hoisting LogicOp where both operands have the same extension op. By default it returns true. On RISC-V, (or disjoint (sext/zext a), (sext/zext b)) can be combined as vwadd.vv/vwaddu.vv. So for such case, it returns false. --- llvm/include/llvm/CodeGen/TargetLowering.h | 10 ++++++++++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 3 +++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 8 ++++++++ llvm/lib/Target/RISCV/RISCVISelLowering.h | 3 +++ llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll | 14 ++++---------- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h index 00c36266a069f..90a1d0e52d8c8 100644 --- a/llvm/include/llvm/CodeGen/TargetLowering.h +++ b/llvm/include/llvm/CodeGen/TargetLowering.h @@ -4462,6 +4462,16 @@ class TargetLowering : public TargetLoweringBase { return false; } + /// Return true if it is profitable to hoist a LogicOp where both operands + /// have the same extension op. This transformation may not be desirable if + /// it disrupts a particularly auspicious target-specific tree (e.g. + /// (or disjoint (zext A), (zext B)) -> vwaddu.wv on RISC-V). By default it + /// returns true. + virtual bool isDesirableToHoistLogicOpWithExt(const SDNode *LogicOp, + unsigned ExtOp) const { + return true; + } + /// Return true if the target supports swifterror attribute. It optimizes /// loads and stores to reading and writing a specific register. virtual bool supportSwiftError() const { diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index b175e35385ec6..6cbf2b6682929 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -5981,6 +5981,9 @@ SDValue DAGCombiner::hoistLogicOpWithSameOpcodeHands(SDNode *N) { HandOpcode == ISD::ANY_EXTEND_VECTOR_INREG) && LegalTypes && !TLI.isTypeDesirableForOp(LogicOpcode, XVT)) return SDValue(); + // If it is not desirable to hoist LogicOp with extension. + if (!TLI.isDesirableToHoistLogicOpWithExt(N, HandOpcode)) + return SDValue(); // logic_op (hand_op X), (hand_op Y) --> hand_op (logic_op X, Y) SDValue Logic = DAG.getNode(LogicOpcode, DL, XVT, X, Y); if (HandOpcode == ISD::SIGN_EXTEND_INREG) diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index 98fba9e86e88a..fe520c0298148 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -19893,6 +19893,14 @@ bool RISCVTargetLowering::isDesirableToCommuteWithShift( return true; } +bool RISCVTargetLowering::isDesirableToHoistLogicOpWithExt( + const SDNode *LogicOp, unsigned ExtOp) const { + if (NodeExtensionHelper::isSupportedRoot(LogicOp, Subtarget) && + (ExtOp == ISD::ZERO_EXTEND || ExtOp == ISD::SIGN_EXTEND)) + return false; + return true; +} + bool RISCVTargetLowering::targetShrinkDemandedConstant( SDValue Op, const APInt &DemandedBits, const APInt &DemandedElts, TargetLoweringOpt &TLO) const { diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h index baf1b2e4d8e6e..63d2fa95d5034 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.h +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h @@ -747,6 +747,9 @@ class RISCVTargetLowering : public TargetLowering { bool isDesirableToCommuteWithShift(const SDNode *N, CombineLevel Level) const override; + bool isDesirableToHoistLogicOpWithExt(const SDNode *LogicOp, + unsigned ExtOp) const override; + /// If a physical register, this returns the register that receives the /// exception address on entry to an EH pad. Register diff --git a/llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll b/llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll index 3f5d42f89337b..149950484c477 100644 --- a/llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll +++ b/llvm/test/CodeGen/RISCV/rvv/vwadd-sdnode.ll @@ -1417,15 +1417,12 @@ define @vwaddu_vv_disjoint_or_add( %x.i8, %add } -; TODO: We could select vwaddu.vv, but when both arms of the or are the same -; DAGCombiner::hoistLogicOpWithSameOpcodeHands moves the zext above the or. define @vwaddu_vv_disjoint_or( %x.i16, %y.i16) { ; CHECK-LABEL: vwaddu_vv_disjoint_or: ; CHECK: # %bb.0: ; CHECK-NEXT: vsetvli a0, zero, e16, mf2, ta, ma -; CHECK-NEXT: vor.vv v9, v8, v9 -; CHECK-NEXT: vsetvli zero, zero, e32, m1, ta, ma -; CHECK-NEXT: vzext.vf2 v8, v9 +; CHECK-NEXT: vwaddu.vv v10, v8, v9 +; CHECK-NEXT: vmv1r.v v8, v10 ; CHECK-NEXT: ret %x.i32 = zext %x.i16 to %y.i32 = zext %y.i16 to @@ -1433,15 +1430,12 @@ define @vwaddu_vv_disjoint_or( %x.i16, %or } -; TODO: We could select vwadd.vv, but when both arms of the or are the same -; DAGCombiner::hoistLogicOpWithSameOpcodeHands moves the zext above the or. define @vwadd_vv_disjoint_or( %x.i16, %y.i16) { ; CHECK-LABEL: vwadd_vv_disjoint_or: ; CHECK: # %bb.0: ; CHECK-NEXT: vsetvli a0, zero, e16, mf2, ta, ma -; CHECK-NEXT: vor.vv v9, v8, v9 -; CHECK-NEXT: vsetvli zero, zero, e32, m1, ta, ma -; CHECK-NEXT: vsext.vf2 v8, v9 +; CHECK-NEXT: vwadd.vv v10, v8, v9 +; CHECK-NEXT: vmv1r.v v8, v10 ; CHECK-NEXT: ret %x.i32 = sext %x.i16 to %y.i32 = sext %y.i16 to From 3b1f46490e18614cacb688bed090cdc2fcf60d5d Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Tue, 22 Apr 2025 21:40:03 +0800 Subject: [PATCH 2/4] Move the condition forward --- llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index fe520c0298148..4efb633cca6c5 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -19895,8 +19895,8 @@ bool RISCVTargetLowering::isDesirableToCommuteWithShift( bool RISCVTargetLowering::isDesirableToHoistLogicOpWithExt( const SDNode *LogicOp, unsigned ExtOp) const { - if (NodeExtensionHelper::isSupportedRoot(LogicOp, Subtarget) && - (ExtOp == ISD::ZERO_EXTEND || ExtOp == ISD::SIGN_EXTEND)) + if ((ExtOp == ISD::ZERO_EXTEND || ExtOp == ISD::SIGN_EXTEND) && + NodeExtensionHelper::isSupportedRoot(LogicOp, Subtarget)) return false; return true; } From a427e702506c5de104aadb954bb21665190bdb4c Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Tue, 22 Apr 2025 22:34:50 +0800 Subject: [PATCH 3/4] Drop the comment --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 6cbf2b6682929..bea96e880d43a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -5981,7 +5981,6 @@ SDValue DAGCombiner::hoistLogicOpWithSameOpcodeHands(SDNode *N) { HandOpcode == ISD::ANY_EXTEND_VECTOR_INREG) && LegalTypes && !TLI.isTypeDesirableForOp(LogicOpcode, XVT)) return SDValue(); - // If it is not desirable to hoist LogicOp with extension. if (!TLI.isDesirableToHoistLogicOpWithExt(N, HandOpcode)) return SDValue(); // logic_op (hand_op X), (hand_op Y) --> hand_op (logic_op X, Y) From 482abae5d59296faf2b8fa2745ca460e398985e7 Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Tue, 22 Apr 2025 22:35:40 +0800 Subject: [PATCH 4/4] Typo: vwaddu.wv -> vwaddu.vv --- llvm/include/llvm/CodeGen/TargetLowering.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h index 90a1d0e52d8c8..6963c091fa2f5 100644 --- a/llvm/include/llvm/CodeGen/TargetLowering.h +++ b/llvm/include/llvm/CodeGen/TargetLowering.h @@ -4465,7 +4465,7 @@ class TargetLowering : public TargetLoweringBase { /// Return true if it is profitable to hoist a LogicOp where both operands /// have the same extension op. This transformation may not be desirable if /// it disrupts a particularly auspicious target-specific tree (e.g. - /// (or disjoint (zext A), (zext B)) -> vwaddu.wv on RISC-V). By default it + /// (or disjoint (zext A), (zext B)) -> vwaddu.vv on RISC-V). By default it /// returns true. virtual bool isDesirableToHoistLogicOpWithExt(const SDNode *LogicOp, unsigned ExtOp) const {