From 45cadb4bd36b479dfc4daff451a6c924703ec7e0 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Wed, 12 Oct 2022 17:49:04 -0700 Subject: [PATCH] [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 --- llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | 113 +++++++++++++++++++----- 1 file changed, 92 insertions(+), 21 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp b/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp index 5f3b295..fd8e6b5 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; } -- 2.7.4