[RISCV] Lower GREVI and GORCI as custom nodes

[frasercrmck]

Summary

This moves the recognition of GREVI and GORCI from TableGen patterns
into a DAGCombine. This is done primarily to match "deeper" patterns in
the future, like (grevi (grevi x, 1) 2) -> (grevi x, 3).

TableGen is not best suited to matching patterns such as these as the compile
time of the DAG matchers quickly gets out of hand due to the expansion of
commutative permutations.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1318

Could we match i32 pre-type legalization on RV64 and type legalize i32 RISCVISD::GREVI to GREVIW? I think that would remove the need to match from SIGN_EXTEND_INREG.

1325

Since this is expected to be a constant, can we use getTargetConstant here and use timm instead of imm in the isel patterns?

1329

Can this just be "return GREV;"?

2818

General question for the RISCV backend, would it make sense to use a macro to generate these cases like X86 and AMDGPU do

#define NODE_NAME_CASE(NODE) case RISCVISD::NODE: return "RISCVISD::" #NODE;

This would eliminate having to mention the node name twice. On X86 I found a couple typos in the strings so having them automatically created can remove human error there.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1318

Good idea; I'll give it a go.

1325

Sounds good to me.

1329

Yes, I think so. Thanks.

2818

I'm glad you brought it up as I'm all in favour of that approach. I can do that as a separate patch to gather feedback.

[frasercrmck]

Address Craig's feedback except:

• using macros; that can be a separate patch
• matching 'W before' legalization; will experiment with that

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1333

Is it possible that the input is (or (or (BITMANIP_SHL x), (BITMANIP_SHR x)), x)?

[frasercrmck]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1318

This idea mostly works but is failing for two RV64 test cases: i32 fshl and fshr intrinsics with constant 16.

fshl is lowered to sign-extended i32 rotl instructions and dag-combined to a (sext_inreg (i64 (or (shl x, 16), (srl (and x, 0xffff0000), 16)))) pattern, skipping the 32-bit or combine. That's something the sext combine was getting but not something the or combine is. Similar for fshr. I think we need the sext in there to be able to match this as a greviw.

Combining certain rotl (and rotr, bswap and bitreverse) to grevi would catch this and would unify everything under one node (something I was mulling about anyway as it'd provide more opportunity for grev combining) but it might also be heavy-handed.

1333

Yeah. It's currently generating this on RV32 master:

  %shl = shl i32 %a, 16
  %shr = lshr i32 %a, 16
  %or = or i32 %shr, %shl
  %or2 = or i32 %or, %a
  ret i32 %or2

-->
  rev16   a1, a0
  or      a0, a1, a0

In that case it's essentially or (grevi x), x) so could in theory be picked up like that rather than fully exploring all potential combinations of (or (or x, y), (or z, w)) inputs.

Sadly though LLVM is picking up that shl/srl as rotl, and still being TableGen pattern matched to grev.

That relates to my other comment, that we might need to dive further into rotl, rotr, bswap and bitreverse to find more opportunities.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1333

Can you post your patch to a separate review? I'd like to experiment with it.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1318

This idea mostly works but is failing for two RV64 test cases: i32 fshl and fshr intrinsics with constant 16.

fshl is lowered to sign-extended i32 rotl instructions and dag-combined to a (sext_inreg (i64 (or (shl x, 16), (srl (and x, 0xffff0000), 16)))) pattern, skipping the 32-bit or combine. That's something the sext combine was getting but not something the or combine is. Similar for fshr. I think we need the sext in there to be able to match this as a greviw.

Combining certain rotl (and rotr, bswap and bitreverse) to grevi would catch this and would unify everything under one node (something I was mulling about anyway as it'd provide more opportunity for grev combining) but it might also be heavy-handed.

I'm not sure that we should be using GREVI for cases that can be handled with ROLW/RORW. I could see a CPU having a better performing implementation of rotate than grevi.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1333

In the absence of time I just sent you an email with the wip patch

[frasercrmck]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1318

Fair enough. Do you think D91449 and D91479 address this problem by catching fshl, fshr, rotl and rotr earlier?

1333

My understanding of what you've done in D91449 would be that it would help us more easily find (or (rorw x), x) and generate a gorc. I think what you've done in removing the reliance on SIGN_EXTEND_INREG is taking our support in a positive direction.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

1333

I think that would still require x to provably have 33 sign bits so that the or propagates the 33 sign bits from rorw. So it might be better to match it while it is still i32 rotr before type legalization.

We'll probably need to match (or (bswap x), x) and (or (bitreverse x), x) before type legalization too.

[frasercrmck]

• Rebase
• Catch W before legalization; legalize to W
• Catch more GORCs
• Adjust fshl/fshr tests expectations

[craig.topper]

LGTM

[frasercrmck]

rebase & fix formatting

llvm/lib/Target/RISCV/RISCVISelLowering.h

enum NodeType : unsigned {
   // is not legal on RV64. FMV_W_X_RV64 matches the semantics of the FMV.W.X.
   // FMV_X_ANYEXTW_RV64 is similar to FMV.X.W but has an any-extended result.
   // This is a more convenient semantic for producing dagcombines that remove
   // unnecessary GPR->FPR->GPR moves.
   FMV_W_X_RV64,
   FMV_X_ANYEXTW_RV64,
   // READ_CYCLE_WIDE - A read of the 64-bit cycle CSR on a 32-bit target
   // (returns (Lo, Hi)). It takes a chain operand.
-  READ_CYCLE_WIDE
+  READ_CYCLE_WIDE,
+  // Generalized Reverse and Generalized Or-Combine - directly matching the
+  // semantics of the named RISC-V instructions. Lowered as custom nodes as
+  // TableGen chokes when faced with commutative permutations in deeply-nested
+  // DAGs. Each node takes an input operand and a TargetConstant immediate
+  // shift amount, and outputs a bit-manipulated version of input. All operands
+  // are of type XLenVT.
+  GREVI,
+  GREVIW,
+  GORCI,
+  GORCIW,
 };
-}
+} // namespace RISCVISD
 
 class RISCVTargetLowering : public TargetLowering {
   const RISCVSubtarget &Subtarget;
 
 public:
   explicit RISCVTargetLowering(const TargetMachine &TM,
                                const RISCVSubtarget &STI);
 

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
   // Effectively disable jump table generation.
   setMinimumJumpTableEntries(INT_MAX);
 
   // Jumps are expensive, compared to logic
   setJumpIsExpensive();
 
   // We can use any register for comparisons
   setHasMultipleConditionRegisters();
+
+  if (Subtarget.hasStdExtZbp()) {
+    setTargetDAGCombine(ISD::OR);
+  }
 }
 
 EVT RISCVTargetLowering::getSetCCResultType(const DataLayout &DL, LLVMContext &,
                                             EVT VT) const {
   if (!VT.isVector())
     return getPointerTy(DL);
   return VT.changeVectorElementTypeToInteger();
 }
static RISCVISD::NodeType getRISCVWOpcode(unsigned Opcode) {
   case ISD::SRL:
     return RISCVISD::SRLW;
   case ISD::SDIV:
     return RISCVISD::DIVW;
   case ISD::UDIV:
     return RISCVISD::DIVUW;
   case ISD::UREM:
     return RISCVISD::REMUW;
+  case RISCVISD::GREVI:
+    return RISCVISD::GREVIW;
+  case RISCVISD::GORCI:
+    return RISCVISD::GORCIW;
   }
 }
 
 // Converts the given 32-bit operation to a target-specific SelectionDAG node.
 // Because i32 isn't a legal type for RV64, these operations would otherwise
 // be promoted to i64, making it difficult to select the SLLW/DIVUW/.../*W
 // later one because the fact the operation was originally of type i32 is
 // lost.
case ISD::BITCAST: {
     SDValue Op0 = N->getOperand(0);
     if (Op0.getValueType() != MVT::f32)
       return;
     SDValue FPConv =
         DAG.getNode(RISCVISD::FMV_X_ANYEXTW_RV64, DL, MVT::i64, Op0);
     Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, FPConv));
     break;
   }
+  case RISCVISD::GREVI:
+  case RISCVISD::GORCI: {
+    assert(N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
+           "Unexpected custom legalisation");
+    // This is similar to customLegalizeToWOp, except that we pass the second
+    // operand (a TargetConstant) straight through: it is already of type
+    // XLenVT.
+    SDLoc DL(N);
+    RISCVISD::NodeType WOpcode = getRISCVWOpcode(N->getOpcode());
+    SDValue NewOp0 =
+        DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, N->getOperand(0));
+    SDValue NewRes =
+        DAG.getNode(WOpcode, DL, MVT::i64, NewOp0, N->getOperand(1));
+    // ReplaceNodeResults requires we maintain the same type for the return
+    // value.
+    Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, NewRes));
+    break;
+  }
+  }
+}
+
+// A structure to hold one of the bit-manipulation patterns below. Together, a
+// SHL and non-SHL pattern may form a bit-manipulation pair on a single source:
+//   (or (and (shl x, 1), 0xAAAAAAAA),
+//       (and (srl x, 1), 0x55555555))
+struct RISCVBitmanipPat {
+  SDValue Op;
+  unsigned ShAmt;
+  bool IsSHL;
+
+  bool formsPairWith(const RISCVBitmanipPat &Other) const {
+    return Op == Other.Op && ShAmt == Other.ShAmt && IsSHL != Other.IsSHL;
+  }
+};
+
+// Matches any of the following bit-manipulation patterns:
+//   (and (shl x, 1), (0x55555555 << 1))
+//   (and (srl x, 1), 0x55555555)
+//   (shl (and x, 0x55555555), 1)
+//   (srl (and x, (0x55555555 << 1)), 1)
+// where the shift amount and mask may vary thus:
+//   [1]  = 0x55555555 / 0xAAAAAAAA
+//   [2]  = 0x33333333 / 0xCCCCCCCC
+//   [4]  = 0x0F0F0F0F / 0xF0F0F0F0
+//   [8]  = 0x00FF00FF / 0xFF00FF00
+//   [16] = 0x0000FFFF / 0xFFFFFFFF
+//   [32] = 0x00000000FFFFFFFF / 0xFFFFFFFF00000000 (for RV64)
+static Optional<RISCVBitmanipPat> matchRISCVBitmanipPat(SDValue Op) {
+  Optional<uint64_t> Mask;
+  // Optionally consume a mask around the shift operation.
+  if (Op.getOpcode() == ISD::AND && isa<ConstantSDNode>(Op.getOperand(1))) {
+    Mask = Op.getConstantOperandVal(1);
+    Op = Op.getOperand(0);
+  }
+  if (Op.getOpcode() != ISD::SHL && Op.getOpcode() != ISD::SRL)
+    return None;
+  bool IsSHL = Op.getOpcode() == ISD::SHL;
+
+  if (!isa<ConstantSDNode>(Op.getOperand(1)))
+    return None;
+  auto ShAmt = Op.getConstantOperandVal(1);
+
+  if (!isPowerOf2_64(ShAmt))
+    return None;
+
+  // These are the unshifted masks which we use to match bit-manipulation
+  // patterns. They may be shifted left in certain circumstances.
+  static const uint64_t BitmanipMasks[] = {
+      0x5555555555555555ULL, 0x3333333333333333ULL, 0x0F0F0F0F0F0F0F0FULL,
+      0x00FF00FF00FF00FFULL, 0x0000FFFF0000FFFFULL, 0x00000000FFFFFFFFULL,
+  };
+
+  unsigned MaskIdx = Log2_64(ShAmt);
+  if (MaskIdx >= array_lengthof(BitmanipMasks))
+    return None;
+
+  auto Src = Op.getOperand(0);
+
+  unsigned Width = Op.getValueType() == MVT::i64 ? 64 : 32;
+  auto ExpMask = BitmanipMasks[MaskIdx] & maskTrailingOnes<uint64_t>(Width);
+
+  // The expected mask is shifted left when the AND is found around SHL
+  // patterns.
+  //   ((x >> 1) & 0x55555555)
+  //   ((x << 1) & 0xAAAAAAAA)
+  bool SHLExpMask = IsSHL;
+
+  if (!Mask) {
+    // Sometimes LLVM keeps the mask as an operand of the shift, typically when
+    // the mask is all ones: consume that now.
+    if (Src.getOpcode() == ISD::AND && isa<ConstantSDNode>(Src.getOperand(1))) {
+      Mask = Src.getConstantOperandVal(1);
+      Src = Src.getOperand(0);
+      // The expected mask is now in fact shifted left for SRL, so reverse the
+      // decision.
+      //   ((x & 0xAAAAAAAA) >> 1)
+      //   ((x & 0x55555555) << 1)
+      SHLExpMask = !SHLExpMask;
+    } else {
+      // Use a default shifted mask of all-ones if there's no AND, truncated
+      // down to the expected width. This simplifies the logic later on.
+      Mask = maskTrailingOnes<uint64_t>(Width);
+      *Mask &= (IsSHL ? *Mask << ShAmt : *Mask >> ShAmt);
+    }
   }
+
+  if (SHLExpMask)
+    ExpMask <<= ShAmt;
+
+  if (Mask != ExpMask)
+    return None;
+
+  return RISCVBitmanipPat{Src, (unsigned)ShAmt, IsSHL};
+}
+
+// Match the following pattern as a GREVI(W) operation
+//   (or (BITMANIP_SHL x), (BITMANIP_SRL x))
+static SDValue combineORToGREV(SDValue Op, SelectionDAG &DAG,
+                               const RISCVSubtarget &Subtarget) {
+  if (Op.getSimpleValueType() == Subtarget.getXLenVT() ||
+      (Subtarget.is64Bit() && Op.getSimpleValueType() == MVT::i32)) {
+    auto LHS = matchRISCVBitmanipPat(Op.getOperand(0));
+    auto RHS = matchRISCVBitmanipPat(Op.getOperand(1));
+    if (LHS && RHS && LHS->formsPairWith(*RHS)) {
+      SDLoc DL(Op);
+      return DAG.getNode(
+          RISCVISD::GREVI, DL, Op.getValueType(), LHS->Op,
+          DAG.getTargetConstant(LHS->ShAmt, DL, Subtarget.getXLenVT()));
+    }
+  }
+  return SDValue();
+}
+
+// Matches any the following pattern as a GORCI(W) operation
+// 1.  (or (GREVI x, shamt), x)
+// 2.  (or x, (GREVI x, shamt))
+// 3.  (or (or (BITMANIP_SHL x), x), (BITMANIP_SRL x))
+// Note that with the variant of 3.,
+//     (or (or (BITMANIP_SHL x), (BITMANIP_SRL x)), x)
+// the inner pattern will first be matched as GREVI and then the outer
+// pattern will be matched to GORC via the first rule above.
+static SDValue combineORToGORC(SDValue Op, SelectionDAG &DAG,
+                               const RISCVSubtarget &Subtarget) {
+  if (Op.getSimpleValueType() == Subtarget.getXLenVT() ||
+      (Subtarget.is64Bit() && Op.getSimpleValueType() == MVT::i32)) {
+    SDLoc DL(Op);
+    SDValue Op0 = Op.getOperand(0);
+    SDValue Op1 = Op.getOperand(1);
+
+    // Check for either commutable permutation of (or (GREVI x, shamt), x)
+    for (const auto &OpPair :
+         {std::make_pair(Op0, Op1), std::make_pair(Op1, Op0)}) {
+      if (OpPair.first.getOpcode() == RISCVISD::GREVI &&
+          OpPair.first.getOperand(0) == OpPair.second)
+        return DAG.getNode(RISCVISD::GORCI, DL, Op.getValueType(),
+                           OpPair.second, OpPair.first.getOperand(1));
+    }
+
+    // OR is commutable so canonicalize its OR operand to the left
+    if (Op0.getOpcode() != ISD::OR && Op1.getOpcode() == ISD::OR)
+      std::swap(Op0, Op1);
+    if (Op0.getOpcode() != ISD::OR)
+      return SDValue();
+    SDValue OrOp0 = Op0.getOperand(0);
+    SDValue OrOp1 = Op0.getOperand(1);
+    auto LHS = matchRISCVBitmanipPat(OrOp0);
+    // OR is commutable so swap the operands and try again: x might have been
+    // on the left
+    if (!LHS) {
+      std::swap(OrOp0, OrOp1);
+      LHS = matchRISCVBitmanipPat(OrOp0);
+    }
+    auto RHS = matchRISCVBitmanipPat(Op1);
+    if (LHS && RHS && LHS->formsPairWith(*RHS) && LHS->Op == OrOp1) {
+      return DAG.getNode(
+          RISCVISD::GORCI, DL, Op.getValueType(), LHS->Op,
+          DAG.getTargetConstant(LHS->ShAmt, DL, Subtarget.getXLenVT()));
+    }
+  }
+  return SDValue();
 }
 
 SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
                                                DAGCombinerInfo &DCI) const {
   SelectionDAG &DAG = DCI.DAG;
 
   switch (N->getOpcode()) {
   default:
case RISCVISD::SRLW: {
     if (SimplifyDemandedBits(N->getOperand(0), LHSMask, DCI) ||
         SimplifyDemandedBits(N->getOperand(1), RHSMask, DCI)) {
       if (N->getOpcode() != ISD::DELETED_NODE)
         DCI.AddToWorklist(N);
       return SDValue(N, 0);
     }
     break;
   }
+  case RISCVISD::GREVIW:
+  case RISCVISD::GORCIW: {
+    // Only the lower 32 bits of the first operand are read
+    SDValue Op0 = N->getOperand(0);
+    APInt Mask = APInt::getLowBitsSet(Op0.getValueSizeInBits(), 32);
+    if (SimplifyDemandedBits(Op0, Mask, DCI)) {
+      if (N->getOpcode() != ISD::DELETED_NODE)
+        DCI.AddToWorklist(N);
+      return SDValue(N, 0);
+    }
+    break;
+  }
   case RISCVISD::FMV_X_ANYEXTW_RV64: {
     SDLoc DL(N);
     SDValue Op0 = N->getOperand(0);
     // If the input to FMV_X_ANYEXTW_RV64 is just FMV_W_X_RV64 then the
     // conversion is unnecessary and can be replaced with an ANY_EXTEND
     // of the FMV_W_X_RV64 operand.
     if (Op0->getOpcode() == RISCVISD::FMV_W_X_RV64) {
       assert(Op0.getOperand(0).getValueType() == MVT::i64 &&
              "Unexpected value type!");
       return Op0.getOperand(0);
     }
 
     // This is a target-specific version of a DAGCombine performed in
     // DAGCombiner::visitBITCAST. It performs the equivalent of:
     // fold (bitconvert (fneg x)) -> (xor (bitconvert x), signbit)
     // fold (bitconvert (fabs x)) -> (and (bitconvert x), (not signbit))
     if (!(Op0.getOpcode() == ISD::FNEG || Op0.getOpcode() == ISD::FABS) ||
         !Op0.getNode()->hasOneUse())
       break;
     SDValue NewFMV = DAG.getNode(RISCVISD::FMV_X_ANYEXTW_RV64, DL, MVT::i64,
                                  Op0.getOperand(0));
     APInt SignBit = APInt::getSignMask(32).sext(64);
     if (Op0.getOpcode() == ISD::FNEG)
       return DAG.getNode(ISD::XOR, DL, MVT::i64, NewFMV,
                          DAG.getConstant(SignBit, DL, MVT::i64));
 
     assert(Op0.getOpcode() == ISD::FABS);
     return DAG.getNode(ISD::AND, DL, MVT::i64, NewFMV,
                        DAG.getConstant(~SignBit, DL, MVT::i64));
   }
+  case ISD::OR:
+    if (auto GREV = combineORToGREV(SDValue(N, 0), DCI.DAG, Subtarget))
+      return GREV;
+    if (auto GORC = combineORToGORC(SDValue(N, 0), DCI.DAG, Subtarget))
+      return GORC;
+    break;
   }
 
   return SDValue();
 }
 
 bool RISCVTargetLowering::isDesirableToCommuteWithShift(
     const SDNode *N, CombineLevel Level) const {
   // The following folds are only desirable if `(OP _, c1 << c2)` can be
   // materialised in fewer instructions than `(OP _, c1)`:
   //
unsigned RISCVTargetLowering::ComputeNumSignBitsForTargetNode(
   default:
     break;
   case RISCVISD::SLLW:
   case RISCVISD::SRAW:
   case RISCVISD::SRLW:
   case RISCVISD::DIVW:
   case RISCVISD::DIVUW:
   case RISCVISD::REMUW:
+  case RISCVISD::GREVIW:
+  case RISCVISD::GORCIW:
     // TODO: As the result is sign-extended, this is conservatively correct. A
     // more precise answer could be calculated for SRAW depending on known
     // bits in the shift amount.
     return 33;
   }
 
   return 1;
 }

 #define NODE_NAME_CASE(NODE)                                                   \
   case RISCVISD::NODE:                                                         \
     return "RISCVISD::" #NODE;
   // clang-format off
   switch ((RISCVISD::NodeType)Opcode) {
   case RISCVISD::FIRST_NUMBER:
     break;
   NODE_NAME_CASE(RET_FLAG)
   NODE_NAME_CASE(URET_FLAG)
   NODE_NAME_CASE(SRET_FLAG)
   NODE_NAME_CASE(MRET_FLAG)
   NODE_NAME_CASE(CALL)
   NODE_NAME_CASE(SELECT_CC)
   NODE_NAME_CASE(BuildPairF64)
   NODE_NAME_CASE(SplitF64)
   NODE_NAME_CASE(TAIL)
   NODE_NAME_CASE(SLLW)
   NODE_NAME_CASE(SRAW)
   NODE_NAME_CASE(SRLW)
   NODE_NAME_CASE(DIVW)
   NODE_NAME_CASE(DIVUW)
   NODE_NAME_CASE(REMUW)
   NODE_NAME_CASE(FMV_W_X_RV64)
   NODE_NAME_CASE(FMV_X_ANYEXTW_RV64)
   NODE_NAME_CASE(READ_CYCLE_WIDE)
+  NODE_NAME_CASE(GREVI)
+  NODE_NAME_CASE(GREVIW)
+  NODE_NAME_CASE(GORCI)
+  NODE_NAME_CASE(GORCIW)
   }
   // clang-format on
   return nullptr;
 #undef NODE_NAME_CASE
 }
 
 /// getConstraintType - Given a constraint letter, return the type of
 /// constraint it is for this target.

llvm/lib/Target/RISCV/RISCVInstrInfoB.td

 }
 
 // We don't pattern-match sbclri[w], sbseti[w], sbinvi[w] because they are
 // pattern-matched by simple andi, ori, and xori.
 let Predicates = [HasStdExtZbs] in
 def : Pat<(and (srl GPR:$rs1, uimmlog2xlen:$shamt), (XLenVT 1)),
           (SBEXTI GPR:$rs1, uimmlog2xlen:$shamt)>;
 
-let Predicates = [HasStdExtZbp, IsRV32] in {
-def : Pat<(or (or (and (srl GPR:$rs1, (i32 1)), (i32 0x55555555)), GPR:$rs1),
-              (and (shl GPR:$rs1, (i32 1)), (i32 0xAAAAAAAA))),
-          (GORCI GPR:$rs1, (i32 1))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i32 2)), (i32 0x33333333)), GPR:$rs1),
-              (and (shl GPR:$rs1, (i32 2)), (i32 0xCCCCCCCC))),
-          (GORCI GPR:$rs1, (i32 2))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i32 4)), (i32 0x0F0F0F0F)), GPR:$rs1),
-              (and (shl GPR:$rs1, (i32 4)), (i32 0xF0F0F0F0))),
-          (GORCI GPR:$rs1, (i32 4))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i32 8)), (i32 0x00FF00FF)), GPR:$rs1),
-              (and (shl GPR:$rs1, (i32 8)), (i32 0xFF00FF00))),
-          (GORCI GPR:$rs1, (i32 8))>;
-def : Pat<(or (or (srl GPR:$rs1, (i32 16)), GPR:$rs1),
-              (shl GPR:$rs1, (i32 16))),
-          (GORCI GPR:$rs1, (i32 16))>;
-} // Predicates = [HasStdExtZbp, IsRV32]
+def SDT_RISCVGREVGORC  : SDTypeProfile<1, 2, [SDTCisVT<0, XLenVT>,
+                                              SDTCisSameAs<0, 1>,
+                                              SDTCisSameAs<1, 2>]>;
+def riscv_grevi    : SDNode<"RISCVISD::GREVI", SDT_RISCVGREVGORC, []>;
+def riscv_greviw   : SDNode<"RISCVISD::GREVIW", SDT_RISCVGREVGORC, []>;
+def riscv_gorci    : SDNode<"RISCVISD::GORCI", SDT_RISCVGREVGORC, []>;
+def riscv_gorciw   : SDNode<"RISCVISD::GORCIW", SDT_RISCVGREVGORC, []>;
 
-let Predicates = [HasStdExtZbp, IsRV64] in {
-def : Pat<(or (or (and (srl GPR:$rs1, (i64 1)), (i64 0x5555555555555555)),
-                   GPR:$rs1),
-              (and (shl GPR:$rs1, (i64 1)), (i64 0xAAAAAAAAAAAAAAAA))),
-          (GORCI GPR:$rs1, (i64 1))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i64 2)), (i64 0x3333333333333333)),
-                   GPR:$rs1),
-              (and (shl GPR:$rs1, (i64 2)), (i64 0xCCCCCCCCCCCCCCCC))),
-          (GORCI GPR:$rs1, (i64 2))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i64 4)), (i64 0x0F0F0F0F0F0F0F0F)),
-                   GPR:$rs1),
-              (and (shl GPR:$rs1, (i64 4)), (i64 0xF0F0F0F0F0F0F0F0))),
-          (GORCI GPR:$rs1, (i64 4))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i64 8)), (i64 0x00FF00FF00FF00FF)),
-                   GPR:$rs1),
-              (and (shl GPR:$rs1, (i64 8)), (i64 0xFF00FF00FF00FF00))),
-          (GORCI GPR:$rs1, (i64 8))>;
-def : Pat<(or (or (and (srl GPR:$rs1, (i64 16)), (i64 0x0000FFFF0000FFFF)),
-                   GPR:$rs1),
-              (and (shl GPR:$rs1, (i64 16)), (i64 0xFFFF0000FFFF0000))),
-          (GORCI GPR:$rs1, (i64 16))>;
-def : Pat<(or (or (srl GPR:$rs1, (i64 32)), GPR:$rs1),
-              (shl GPR:$rs1, (i64 32))),
-          (GORCI GPR:$rs1, (i64 32))>;
-} // Predicates = [HasStdExtZbp, IsRV64]
+let Predicates = [HasStdExtZbp] in {
+def : Pat<(riscv_grevi GPR:$rs1, timm:$shamt), (GREVI GPR:$rs1, timm:$shamt)>;
+def : Pat<(riscv_gorci GPR:$rs1, timm:$shamt), (GORCI GPR:$rs1, timm:$shamt)>;
+} // Predicates = [HasStdExtZbp]
 
 let Predicates = [HasStdExtZbp, IsRV32] in {
-def : Pat<(or (and (shl GPR:$rs1, (i32 1)), (i32 0xAAAAAAAA)),
-              (and (srl GPR:$rs1, (i32 1)), (i32 0x55555555))),
-          (GREVI GPR:$rs1, (i32 1))>;
-def : Pat<(or (and (shl GPR:$rs1, (i32 2)), (i32 0xCCCCCCCC)),
-              (and (srl GPR:$rs1, (i32 2)), (i32 0x33333333))),
-          (GREVI GPR:$rs1, (i32 2))>;
-def : Pat<(or (and (shl GPR:$rs1, (i32 4)), (i32 0xF0F0F0F0)),
-              (and (srl GPR:$rs1, (i32 4)), (i32 0x0F0F0F0F))),
-          (GREVI GPR:$rs1, (i32 4))>;
-def : Pat<(or (and (shl GPR:$rs1, (i32 8)), (i32 0xFF00FF00)),
-              (and (srl GPR:$rs1, (i32 8)), (i32 0x00FF00FF))),
-          (GREVI GPR:$rs1, (i32 8))>;
 def : Pat<(rotr (bswap GPR:$rs1), (i32 16)), (GREVI GPR:$rs1, (i32 8))>;
 // FIXME: Is grev better than rori?
 def : Pat<(rotl GPR:$rs1, (i32 16)), (GREVI GPR:$rs1, (i32 16))>;
 def : Pat<(rotr GPR:$rs1, (i32 16)), (GREVI GPR:$rs1, (i32 16))>;
 def : Pat<(bswap GPR:$rs1), (GREVI GPR:$rs1, (i32 24))>;
 def : Pat<(bitreverse GPR:$rs1), (GREVI GPR:$rs1, (i32 31))>;
 } // Predicates = [HasStdExtZbp, IsRV32]
 
 let Predicates = [HasStdExtZbp, IsRV64] in {
-def : Pat<(or (and (shl GPR:$rs1, (i64 1)), (i64 0xAAAAAAAAAAAAAAAA)),
-              (and (srl GPR:$rs1, (i64 1)), (i64 0x5555555555555555))),
-          (GREVI GPR:$rs1, (i64 1))>;
-def : Pat<(or (and (shl GPR:$rs1, (i64 2)), (i64 0xCCCCCCCCCCCCCCCC)),
-              (and (srl GPR:$rs1, (i64 2)), (i64 0x3333333333333333))),
-          (GREVI GPR:$rs1, (i64 2))>;
-def : Pat<(or (and (shl GPR:$rs1, (i64 4)), (i64 0xF0F0F0F0F0F0F0F0)),
-              (and (srl GPR:$rs1, (i64 4)), (i64 0x0F0F0F0F0F0F0F0F))),
-          (GREVI GPR:$rs1, (i64 4))>;
-def : Pat<(or (and (shl GPR:$rs1, (i64 8)), (i64 0xFF00FF00FF00FF00)),
-              (and (srl GPR:$rs1, (i64 8)), (i64 0x00FF00FF00FF00FF))),
-          (GREVI GPR:$rs1, (i64 8))>;
-def : Pat<(or (and (shl GPR:$rs1, (i64 16)), (i64 0xFFFF0000FFFF0000)),
-              (and (srl GPR:$rs1, (i64 16)), (i64 0x0000FFFF0000FFFF))),
-          (GREVI GPR:$rs1, (i64 16))>;
 // FIXME: Is grev better than rori?
 def : Pat<(rotl GPR:$rs1, (i64 32)), (GREVI GPR:$rs1, (i64 32))>;
 def : Pat<(rotr GPR:$rs1, (i64 32)), (GREVI GPR:$rs1, (i64 32))>;
 def : Pat<(bswap GPR:$rs1), (GREVI GPR:$rs1, (i64 56))>;
 def : Pat<(bitreverse GPR:$rs1), (GREVI GPR:$rs1, (i64 63))>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
 let Predicates = [HasStdExtZbt] in {
def : Pat<(SROIWPat GPR:$rs1, uimmlog2xlen:$shamt),
           (SROIW GPR:$rs1, uimmlog2xlen:$shamt)>;
 } // Predicates = [HasStdExtZbb, IsRV64]
 
 let Predicates = [HasStdExtZbbOrZbp, IsRV64] in
 def : Pat<(RORIWPat GPR:$rs1, uimmlog2xlen:$shamt),
           (RORIW GPR:$rs1, uimmlog2xlen:$shamt)>;
 
 let Predicates = [HasStdExtZbp, IsRV64] in {
-def : Pat<(sext_inreg (or (or (and (srl GPR:$rs1, (i64 1)), (i64 0x55555555)),
-                              GPR:$rs1),
-                          (and (shl GPR:$rs1, (i64 1)), (i64 0xAAAAAAAA))),
-                      i32),
-          (GORCIW GPR:$rs1, (i64 1))>;
-def : Pat<(sext_inreg (or (or (and (srl GPR:$rs1, (i64 2)), (i64 0x33333333)),
-                              GPR:$rs1),
-                          (and (shl GPR:$rs1, (i64 2)), (i64 0xCCCCCCCC))),
-                      i32),
-          (GORCIW GPR:$rs1, (i64 2))>;
-def : Pat<(sext_inreg (or (or (and (srl GPR:$rs1, (i64 4)), (i64 0x0F0F0F0F)),
-                              GPR:$rs1),
-                          (and (shl GPR:$rs1, (i64 4)), (i64 0xF0F0F0F0))),
-                      i32),
-          (GORCIW GPR:$rs1, (i64 4))>;
-def : Pat<(sext_inreg (or (or (and (srl GPR:$rs1, (i64 8)), (i64 0x00FF00FF)),
-                              GPR:$rs1),
-                          (and (shl GPR:$rs1, (i64 8)), (i64 0xFF00FF00))),
-                      i32),
-          (GORCIW GPR:$rs1, (i64 8))>;
-def : Pat<(sext_inreg (or (or (and (srl GPR:$rs1, (i64 16)), (i64 0x0000FFFF)),
-                              GPR:$rs1),
-                          (and (shl GPR:$rs1, (i64 16)), (i64 0xFFFF0000))),
-                      i32),
-          (GORCIW GPR:$rs1, (i64 16))>;
-def : Pat<(sext_inreg (or (or (srl (and GPR:$rs1, (i64 0xFFFF0000)), (i64 16)),
-                              GPR:$rs1),
-                          (shl GPR:$rs1, (i64 16))), i32),
-          (GORCIW GPR:$rs1, (i64 16))>;
+def : Pat<(riscv_greviw GPR:$rs1, timm:$shamt), (GREVIW GPR:$rs1, timm:$shamt)>;
+def : Pat<(riscv_gorciw GPR:$rs1, timm:$shamt), (GORCIW GPR:$rs1, timm:$shamt)>;
 
-def : Pat<(sext_inreg (or (and (shl GPR:$rs1, (i64 1)), (i64 0xAAAAAAAA)),
-                          (and (srl GPR:$rs1, (i64 1)), (i64 0x55555555))),
-                      i32),
-          (GREVIW GPR:$rs1, (i64 1))>;
-def : Pat<(sext_inreg (or (and (shl GPR:$rs1, (i64 2)), (i64 0xCCCCCCCC)),
-                          (and (srl GPR:$rs1, (i64 2)), (i64 0x33333333))),
-                      i32),
-          (GREVIW GPR:$rs1, (i64 2))>;
-def : Pat<(sext_inreg (or (and (shl GPR:$rs1, (i64 4)), (i64 0xF0F0F0F0)),
-                          (and (srl GPR:$rs1, (i64 4)), (i64 0x0F0F0F0F))),
-                      i32),
-          (GREVIW GPR:$rs1, (i64 4))>;
-def : Pat<(sext_inreg (or (and (shl GPR:$rs1, (i64 8)), (i64 0xFF00FF00)),
-                          (and (srl GPR:$rs1, (i64 8)), (i64 0x00FF00FF))),
-                      i32),
-          (GREVIW GPR:$rs1, (i64 8))>;
-def : Pat<(sext_inreg (or (shl GPR:$rs1, (i64 16)),
-                          (srl (and GPR:$rs1, 0xFFFF0000), (i64 16))), i32),
-          (GREVIW GPR:$rs1, (i64 16))>;
 def : Pat<(sra (bswap GPR:$rs1), (i64 32)), (GREVIW GPR:$rs1, (i64 24))>;
 def : Pat<(sra (bitreverse GPR:$rs1), (i64 32)), (GREVIW GPR:$rs1, (i64 31))>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
 let Predicates = [HasStdExtZbt, IsRV64] in {
 def : Pat<(sext_inreg (fshl GPR:$rs1, (shl GPR:$rs3, (i64 32)),
                             (and GPR:$rs2, (i64 31))),
                       i32),

llvm/test/CodeGen/RISCV/rv64Zbp.ll

 ; RV64I-NEXT:    srliw a1, a0, 16
 ; RV64I-NEXT:    slli a0, a0, 16
 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    sext.w a0, a0
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: grev16_i32_fshl:
 ; RV64IB:       # %bb.0:
-; RV64IB-NEXT:    greviw a0, a0, 16
+; RV64IB-NEXT:    roriw a0, a0, 16
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBP-LABEL: grev16_i32_fshl:
 ; RV64IBP:       # %bb.0:
-; RV64IBP-NEXT:    greviw a0, a0, 16
+; RV64IBP-NEXT:    roriw a0, a0, 16
 ; RV64IBP-NEXT:    ret
   %or = tail call i32 @llvm.fshl.i32(i32 %a, i32 %a, i32 16)
   ret i32 %or
 }
 
 define signext i32 @grev16_i32_fshr(i32 signext %a) nounwind {
 ; RV64I-LABEL: grev16_i32_fshr:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    slli a1, a0, 16
 ; RV64I-NEXT:    srliw a0, a0, 16
 ; RV64I-NEXT:    or a0, a0, a1
 ; RV64I-NEXT:    sext.w a0, a0
 ; RV64I-NEXT:    ret
 ;
 ; RV64IB-LABEL: grev16_i32_fshr:
 ; RV64IB:       # %bb.0:
-; RV64IB-NEXT:    greviw a0, a0, 16
+; RV64IB-NEXT:    roriw a0, a0, 16
 ; RV64IB-NEXT:    ret
 ;
 ; RV64IBP-LABEL: grev16_i32_fshr:
 ; RV64IBP:       # %bb.0:
-; RV64IBP-NEXT:    greviw a0, a0, 16
+; RV64IBP-NEXT:    roriw a0, a0, 16
 ; RV64IBP-NEXT:    ret
   %or = tail call i32 @llvm.fshr.i32(i32 %a, i32 %a, i32 16)
   ret i32 %or
 }
 
 define i64 @grev16_i64(i64 %a) nounwind {
 ; RV64I-LABEL: grev16_i64:
 ; RV64I:       # %bb.0: