[llvm] 45cadb4 - [AArch64][NFC]Refactor 'isBitfieldPositioningOp' so that DAG nodes with different Opcode are handled with separate helper functions.
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 17 08:10:02 PDT 2022
Author: Mingming Liu
Date: 2022-10-17T08:08:48-07:00
New Revision: 45cadb4bd36b479dfc4daff451a6c924703ec7e0
URL: https://github.com/llvm/llvm-project/commit/45cadb4bd36b479dfc4daff451a6c924703ec7e0
DIFF: https://github.com/llvm/llvm-project/commit/45cadb4bd36b479dfc4daff451a6c924703ec7e0.diff
LOG: [AArch64][NFC]Refactor 'isBitfieldPositioningOp' so that DAG nodes with different Opcode are handled with separate helper functions.
Using different helper functions for DAG nodes with different Opcode allows specialization.
- 'isBitfieldExtractOp' [1] shows how specialization based on Opcode could catch more patterns.
- The refactor paves the way (e.g., makes diff clearer) for enhancement in {D135844,D135850,D135852}
[1] https://github.com/llvm/llvm-project/blob/cbd8464595220b5ea76c70ac9965d84970c4b712/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp#L2163-L2202
Differential Revision: https://reviews.llvm.org/D135843
Added:
Modified:
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp b/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
index 5f3b2950ff86..fd8e6b59e036 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
@@ -2495,12 +2495,25 @@ static SDValue getLeftShift(SelectionDAG *CurDAG, SDValue Op, int ShlAmount) {
return SDValue(ShiftNode, 0);
}
+// For bit-field-positioning pattern "(and (shl VAL, N), ShiftedMask)".
+static bool isBitfieldPositioningOpFromAnd(SelectionDAG *CurDAG, SDValue Op,
+ bool BiggerPattern,
+ const uint64_t NonZeroBits,
+ SDValue &Src, int &DstLSB,
+ int &Width);
+
+// For bit-field-positioning pattern "shl VAL, N)".
+static bool isBitfieldPositioningOpFromShl(SelectionDAG *CurDAG, SDValue Op,
+ bool BiggerPattern,
+ const uint64_t NonZeroBits,
+ SDValue &Src, int &DstLSB,
+ int &Width);
+
/// Does this tree qualify as an attempt to move a bitfield into position,
-/// essentially "(and (shl VAL, N), Mask)".
+/// essentially "(and (shl VAL, N), Mask)" or (shl VAL, N).
static bool isBitfieldPositioningOp(SelectionDAG *CurDAG, SDValue Op,
- bool BiggerPattern,
- SDValue &Src, int &ShiftAmount,
- int &MaskWidth) {
+ bool BiggerPattern, SDValue &Src,
+ int &DstLSB, int &Width) {
EVT VT = Op.getValueType();
unsigned BitWidth = VT.getSizeInBits();
(void)BitWidth;
@@ -2510,41 +2523,99 @@ static bool isBitfieldPositioningOp(SelectionDAG *CurDAG, SDValue Op,
// Non-zero in the sense that they're not provably zero, which is the key
// point if we want to use this value
- uint64_t NonZeroBits = (~Known.Zero).getZExtValue();
+ const uint64_t NonZeroBits = (~Known.Zero).getZExtValue();
+ if (!isShiftedMask_64(NonZeroBits))
+ return false;
- // Discard a constant AND mask if present. It's safe because the node will
- // already have been factored into the computeKnownBits calculation above.
- uint64_t AndImm;
- if (isOpcWithIntImmediate(Op.getNode(), ISD::AND, AndImm)) {
- assert((~APInt(BitWidth, AndImm) & ~Known.Zero) == 0);
- Op = Op.getOperand(0);
+ switch (Op.getOpcode()) {
+ default:
+ break;
+ case ISD::AND:
+ return isBitfieldPositioningOpFromAnd(CurDAG, Op, BiggerPattern,
+ NonZeroBits, Src, DstLSB, Width);
+ case ISD::SHL:
+ return isBitfieldPositioningOpFromShl(CurDAG, Op, BiggerPattern,
+ NonZeroBits, Src, DstLSB, Width);
}
- // Don't match if the SHL has more than one use, since then we'll end up
- // generating SHL+UBFIZ instead of just keeping SHL+AND.
- if (!BiggerPattern && !Op.hasOneUse())
+ return false;
+}
+
+static bool isBitfieldPositioningOpFromAnd(SelectionDAG *CurDAG, SDValue Op,
+ bool BiggerPattern,
+ const uint64_t NonZeroBits,
+ SDValue &Src, int &DstLSB,
+ int &Width) {
+ assert(isShiftedMask_64(NonZeroBits) && "Caller guaranteed");
+
+ EVT VT = Op.getValueType();
+ assert((VT == MVT::i32 || VT == MVT::i64) &&
+ "Caller guarantees VT is one of i32 or i64");
+
+ uint64_t AndImm;
+ if (!isOpcWithIntImmediate(Op.getNode(), ISD::AND, AndImm))
return false;
+ // If (~AndImm & NonZeroBits) is not zero at POS, we know that
+ // 1) (AndImm & (1 << POS) == 0)
+ // 2) the result of AND is not zero at POS bit (according to NonZeroBits)
+ //
+ // 1) and 2) don't agree so something must be wrong (e.g., in
+ // 'SelectionDAG::computeKnownBits')
+ assert((~AndImm & NonZeroBits) == 0 &&
+ "Something must be wrong (e.g., in SelectionDAG::computeKnownBits)");
+
+ SDValue AndOp0 = Op.getOperand(0);
+
uint64_t ShlImm;
- if (!isOpcWithIntImmediate(Op.getNode(), ISD::SHL, ShlImm))
+ if (!isOpcWithIntImmediate(AndOp0.getNode(), ISD::SHL, ShlImm))
return false;
- Op = Op.getOperand(0);
- if (!isShiftedMask_64(NonZeroBits))
+ // Bail out if the SHL has more than one use, since then we'll end up
+ // generating SHL+UBFIZ instead of just keeping SHL+AND.
+ if (!BiggerPattern && !AndOp0.hasOneUse())
return false;
- ShiftAmount = countTrailingZeros(NonZeroBits);
- MaskWidth = countTrailingOnes(NonZeroBits >> ShiftAmount);
+ DstLSB = countTrailingZeros(NonZeroBits);
+ Width = countTrailingOnes(NonZeroBits >> DstLSB);
// BFI encompasses sufficiently many nodes that it's worth inserting an extra
// LSL/LSR if the mask in NonZeroBits doesn't quite match up with the ISD::SHL
// amount. BiggerPattern is true when this pattern is being matched for BFI,
// BiggerPattern is false when this pattern is being matched for UBFIZ, in
// which case it is not profitable to insert an extra shift.
- if (ShlImm - ShiftAmount != 0 && !BiggerPattern)
+ if (ShlImm != DstLSB && !BiggerPattern)
+ return false;
+
+ Src = getLeftShift(CurDAG, AndOp0.getOperand(0), ShlImm - DstLSB);
+ return true;
+}
+
+static bool isBitfieldPositioningOpFromShl(SelectionDAG *CurDAG, SDValue Op,
+ bool BiggerPattern,
+ const uint64_t NonZeroBits,
+ SDValue &Src, int &DstLSB,
+ int &Width) {
+ assert(isShiftedMask_64(NonZeroBits) && "Caller guaranteed");
+
+ EVT VT = Op.getValueType();
+ assert((VT == MVT::i32 || VT == MVT::i64) &&
+ "Caller guarantees that type is i32 or i64");
+
+ uint64_t ShlImm;
+ if (!isOpcWithIntImmediate(Op.getNode(), ISD::SHL, ShlImm))
+ return false;
+
+ if (!BiggerPattern && !Op.hasOneUse())
+ return false;
+
+ DstLSB = countTrailingZeros(NonZeroBits);
+ Width = countTrailingOnes(NonZeroBits >> DstLSB);
+
+ if (DstLSB != ShlImm && !BiggerPattern)
return false;
- Src = getLeftShift(CurDAG, Op, ShlImm - ShiftAmount);
+ Src = getLeftShift(CurDAG, Op.getOperand(0), ShlImm - DstLSB);
return true;
}
More information about the llvm-commits
mailing list