[llvm-branch-commits] [llvm] ae88d88 - [SDAG] move x86 select-with-identity-constant fold behind a target hook; NFC
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Feb 14 14:15:34 PST 2022
Author: Sanjay Patel
Date: 2022-02-14T14:10:53-08:00
New Revision: ae88d8844673fbe7a2b7550a8cd168c7bebca99d
URL: https://github.com/llvm/llvm-project/commit/ae88d8844673fbe7a2b7550a8cd168c7bebca99d
DIFF: https://github.com/llvm/llvm-project/commit/ae88d8844673fbe7a2b7550a8cd168c7bebca99d.diff
LOG: [SDAG] move x86 select-with-identity-constant fold behind a target hook; NFC
This is no-functional-change-intended because only the
x86 target enables the TLI hook currently.
We can add fmul/fdiv opcodes to the switch similar to the
proposal D119111, but we don't need to make other changes
like enabling target-specific combines.
We can also add integer opcodes (add, or, shl, etc.) to
the switch because this function is called from all of the
generic binary opcodes.
The goal is to incrementally enable the profitable diffs
from D90113 while avoiding regressions.
Differential Revision: https://reviews.llvm.org/D119150
(cherry picked from commit a68e098024708cba7b37d6c5a27a4003e8944142)
Added:
Modified:
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 3861648a5feba..de8b0e9cfd10b 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -2851,6 +2851,14 @@ class TargetLoweringBase {
return false;
}
+ /// Return true if pulling a binary operation into a select with an identity
+ /// constant is profitable. This is the inverse of an IR transform.
+ /// Example: X + (Cond ? Y : 0) --> Cond ? (X + Y) : X
+ virtual bool shouldFoldSelectWithIdentityConstant(unsigned BinOpcode,
+ EVT VT) const {
+ return false;
+ }
+
/// Return true if it is beneficial to convert a load of a constant to
/// just the constant itself.
/// On some targets it might be more efficient to use a combination of
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 041d7e5b4a4aa..6a36b7862ce95 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -2101,10 +2101,77 @@ static bool canFoldInAddressingMode(SDNode *N, SDNode *Use, SelectionDAG &DAG,
VT.getTypeForEVT(*DAG.getContext()), AS);
}
+/// This inverts a canonicalization in IR that replaces a variable select arm
+/// with an identity constant. Codegen improves if we re-use the variable
+/// operand rather than load a constant. This can also be converted into a
+/// masked vector operation if the target supports it.
+static SDValue foldSelectWithIdentityConstant(SDNode *N, SelectionDAG &DAG,
+ bool ShouldCommuteOperands) {
+ // Match a select as operand 1. The identity constant that we are looking for
+ // is only valid as operand 1 of a non-commutative binop.
+ SDValue N0 = N->getOperand(0);
+ SDValue N1 = N->getOperand(1);
+ if (ShouldCommuteOperands)
+ std::swap(N0, N1);
+
+ // TODO: Should this apply to scalar select too?
+ if (!N1.hasOneUse() || N1.getOpcode() != ISD::VSELECT)
+ return SDValue();
+
+ unsigned Opcode = N->getOpcode();
+ EVT VT = N->getValueType(0);
+ SDValue Cond = N1.getOperand(0);
+ SDValue TVal = N1.getOperand(1);
+ SDValue FVal = N1.getOperand(2);
+
+ // TODO: The cases should match with IR's ConstantExpr::getBinOpIdentity().
+ // TODO: Target-specific opcodes could be added. Ex: "isCommutativeBinOp()".
+ // TODO: With fast-math (NSZ), allow the opposite-sign form of zero?
+ auto isIdentityConstantForOpcode = [](unsigned Opcode, SDValue V) {
+ if (ConstantFPSDNode *C = isConstOrConstSplatFP(V)) {
+ switch (Opcode) {
+ case ISD::FADD: // X + -0.0 --> X
+ return C->isZero() && C->isNegative();
+ case ISD::FSUB: // X - 0.0 --> X
+ return C->isZero() && !C->isNegative();
+ }
+ }
+ return false;
+ };
+
+ // This transform increases uses of N0, so freeze it to be safe.
+ // binop N0, (vselect Cond, IDC, FVal) --> vselect Cond, N0, (binop N0, FVal)
+ if (isIdentityConstantForOpcode(Opcode, TVal)) {
+ SDValue F0 = DAG.getFreeze(N0);
+ SDValue NewBO = DAG.getNode(Opcode, SDLoc(N), VT, F0, FVal, N->getFlags());
+ return DAG.getSelect(SDLoc(N), VT, Cond, F0, NewBO);
+ }
+ // binop N0, (vselect Cond, TVal, IDC) --> vselect Cond, (binop N0, TVal), N0
+ if (isIdentityConstantForOpcode(Opcode, FVal)) {
+ SDValue F0 = DAG.getFreeze(N0);
+ SDValue NewBO = DAG.getNode(Opcode, SDLoc(N), VT, F0, TVal, N->getFlags());
+ return DAG.getSelect(SDLoc(N), VT, Cond, NewBO, F0);
+ }
+
+ return SDValue();
+}
+
SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
assert(TLI.isBinOp(BO->getOpcode()) && BO->getNumValues() == 1 &&
"Unexpected binary operator");
+ const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+ auto BinOpcode = BO->getOpcode();
+ EVT VT = BO->getValueType(0);
+ if (TLI.shouldFoldSelectWithIdentityConstant(BinOpcode, VT)) {
+ if (SDValue Sel = foldSelectWithIdentityConstant(BO, DAG, false))
+ return Sel;
+
+ if (TLI.isCommutativeBinOp(BO->getOpcode()))
+ if (SDValue Sel = foldSelectWithIdentityConstant(BO, DAG, true))
+ return Sel;
+ }
+
// Don't do this unless the old select is going away. We want to eliminate the
// binary operator, not replace a binop with a select.
// TODO: Handle ISD::SELECT_CC.
@@ -2133,7 +2200,6 @@ SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
// propagate non constant operands into select. I.e.:
// and (select Cond, 0, -1), X --> select Cond, 0, X
// or X, (select Cond, -1, 0) --> select Cond, -1, X
- auto BinOpcode = BO->getOpcode();
bool CanFoldNonConst =
(BinOpcode == ISD::AND || BinOpcode == ISD::OR) &&
(isNullOrNullSplat(CT) || isAllOnesOrAllOnesSplat(CT)) &&
@@ -2145,8 +2211,6 @@ SDValue DAGCombiner::foldBinOpIntoSelect(SDNode *BO) {
!DAG.isConstantFPBuildVectorOrConstantFP(CBO))
return SDValue();
- EVT VT = BO->getValueType(0);
-
// We have a select-of-constants followed by a binary operator with a
// constant. Eliminate the binop by pulling the constant math into the select.
// Example: add (select Cond, CT, CF), CBO --> select Cond, CT + CBO, CF + CBO
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 6f1fe81955952..a1c387574ebb7 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -33418,6 +33418,20 @@ bool X86TargetLowering::isNarrowingProfitable(EVT VT1, EVT VT2) const {
return !(VT1 == MVT::i32 && VT2 == MVT::i16);
}
+bool X86TargetLowering::shouldFoldSelectWithIdentityConstant(unsigned Opcode,
+ EVT VT) const {
+ // TODO: This is too general. There are cases where pre-AVX512 codegen would
+ // benefit. The transform may also be profitable for scalar code.
+ if (!Subtarget.hasAVX512())
+ return false;
+ if (!Subtarget.hasVLX() && !VT.is512BitVector())
+ return false;
+ if (!VT.isVector())
+ return false;
+
+ return true;
+}
+
/// Targets can use this to indicate that they only support *some*
/// VECTOR_SHUFFLE operations, those with specific masks.
/// By default, if a target supports the VECTOR_SHUFFLE node, all mask values
@@ -48920,83 +48934,6 @@ static SDValue combineFaddCFmul(SDNode *N, SelectionDAG &DAG,
return DAG.getBitcast(VT, CFmul);
}
-/// This inverts a canonicalization in IR that replaces a variable select arm
-/// with an identity constant. Codegen improves if we re-use the variable
-/// operand rather than load a constant. This can also be converted into a
-/// masked vector operation if the target supports it.
-static SDValue foldSelectWithIdentityConstant(SDNode *N, SelectionDAG &DAG,
- bool ShouldCommuteOperands) {
- // Match a select as operand 1. The identity constant that we are looking for
- // is only valid as operand 1 of a non-commutative binop.
- SDValue N0 = N->getOperand(0);
- SDValue N1 = N->getOperand(1);
- if (ShouldCommuteOperands)
- std::swap(N0, N1);
-
- // TODO: Should this apply to scalar select too?
- if (!N1.hasOneUse() || N1.getOpcode() != ISD::VSELECT)
- return SDValue();
-
- unsigned Opcode = N->getOpcode();
- EVT VT = N->getValueType(0);
- SDValue Cond = N1.getOperand(0);
- SDValue TVal = N1.getOperand(1);
- SDValue FVal = N1.getOperand(2);
-
- // TODO: This (and possibly the entire function) belongs in a
- // target-independent location with target hooks.
- // TODO: The cases should match with IR's ConstantExpr::getBinOpIdentity().
- // TODO: With fast-math (NSZ), allow the opposite-sign form of zero?
- auto isIdentityConstantForOpcode = [](unsigned Opcode, SDValue V) {
- if (ConstantFPSDNode *C = isConstOrConstSplatFP(V)) {
- switch (Opcode) {
- case ISD::FADD: // X + -0.0 --> X
- return C->isZero() && C->isNegative();
- case ISD::FSUB: // X - 0.0 --> X
- return C->isZero() && !C->isNegative();
- }
- }
- return false;
- };
-
- // This transform increases uses of N0, so freeze it to be safe.
- // binop N0, (vselect Cond, IDC, FVal) --> vselect Cond, N0, (binop N0, FVal)
- if (isIdentityConstantForOpcode(Opcode, TVal)) {
- SDValue F0 = DAG.getFreeze(N0);
- SDValue NewBO = DAG.getNode(Opcode, SDLoc(N), VT, F0, FVal, N->getFlags());
- return DAG.getSelect(SDLoc(N), VT, Cond, F0, NewBO);
- }
- // binop N0, (vselect Cond, TVal, IDC) --> vselect Cond, (binop N0, TVal), N0
- if (isIdentityConstantForOpcode(Opcode, FVal)) {
- SDValue F0 = DAG.getFreeze(N0);
- SDValue NewBO = DAG.getNode(Opcode, SDLoc(N), VT, F0, TVal, N->getFlags());
- return DAG.getSelect(SDLoc(N), VT, Cond, NewBO, F0);
- }
-
- return SDValue();
-}
-
-static SDValue combineBinopWithSelect(SDNode *N, SelectionDAG &DAG,
- const X86Subtarget &Subtarget) {
- // TODO: This is too general. There are cases where pre-AVX512 codegen would
- // benefit. The transform may also be profitable for scalar code.
- if (!Subtarget.hasAVX512())
- return SDValue();
-
- if (!Subtarget.hasVLX() && !N->getValueType(0).is512BitVector())
- return SDValue();
-
- if (SDValue Sel = foldSelectWithIdentityConstant(N, DAG, false))
- return Sel;
-
- const TargetLowering &TLI = DAG.getTargetLoweringInfo();
- if (TLI.isCommutativeBinOp(N->getOpcode()))
- if (SDValue Sel = foldSelectWithIdentityConstant(N, DAG, true))
- return Sel;
-
- return SDValue();
-}
-
/// Do target-specific dag combines on floating-point adds/subs.
static SDValue combineFaddFsub(SDNode *N, SelectionDAG &DAG,
const X86Subtarget &Subtarget) {
@@ -49006,9 +48943,6 @@ static SDValue combineFaddFsub(SDNode *N, SelectionDAG &DAG,
if (SDValue COp = combineFaddCFmul(N, DAG, Subtarget))
return COp;
- if (SDValue Sel = combineBinopWithSelect(N, DAG, Subtarget))
- return Sel;
-
return SDValue();
}
diff --git a/llvm/lib/Target/X86/X86ISelLowering.h b/llvm/lib/Target/X86/X86ISelLowering.h
index 3f6d567d3f4de..50c7e2c319f63 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.h
+++ b/llvm/lib/Target/X86/X86ISelLowering.h
@@ -1288,6 +1288,9 @@ namespace llvm {
/// from i32 to i8 but not from i32 to i16.
bool isNarrowingProfitable(EVT VT1, EVT VT2) const override;
+ bool shouldFoldSelectWithIdentityConstant(unsigned BinOpcode,
+ EVT VT) const override;
+
/// Given an intrinsic, checks if on the target the intrinsic will need to map
/// to a MemIntrinsicNode (touches memory). If this is the case, it returns
/// true and stores the intrinsic information into the IntrinsicInfo that was
More information about the llvm-branch-commits
mailing list