[RISCV] Support @llvm.readcyclecounter() Intrinsic

[lenary]

Summary

On RISC-V, the cycle CSR holds a 64-bit count of the number of clock
cycles executed by the core, from an arbitrary point in the past. This
matches the intended semantics of @llvm.readcyclecounter(), which we
currently leave to the default lowering (to the constant 0).

With this patch, we will now correctly lower this intrinsic to the
intended semantics, using the user-space instruction rdcycle. On
64-bit targets, we can directly lower to this instruction.

On 32-bit targets, we need to do more, as rdcycle only returns the low
32-bits of the cycle CSR. In this case, we perform a custom lowering,
based on the PowerPC lowering, using rdcycleh to obtain the high
32-bits of the cycle CSR. This custom lowering inserts a new basic
block which detects overflow in the high 32-bits of the cycle CSR
during reading (because multiple instructions are required to read). The
emitted assembly matches the suggested assembly in the RISC-V
specification.

[lebedev.ri]

llvm/test/CodeGen/RISCV/readcyclecounter.ll

20 ↗

(On Diff #207737)

If you change

-define i64 @test_builtin_readcyclecounter() {
+define i64 @test_builtin_readcyclecounter() nounwind {

you should get rid of this CFI noise

[lenary]

• Update test to add nounwind.

[lenary]

llvm/test/CodeGen/RISCV/readcyclecounter.ll

20 ↗

(On Diff #207737)

Thanks for the tip!

[lenary]

• Run Clang-Format

[asb]

Thanks Sam - left a number of comments in-line.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

160 ↗

(On Diff #207748)

Couldn't hurt to add an assert here that that !Subtarget.is64Bit()

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

183 ↗

(On Diff #207748)

Nit: should have full stop at end of comment

843 ↗

(On Diff #207748)

Should add an assert that !Subtarget.is64Bit()

845 ↗

(On Diff #207748)

RTB isn't really a meangful name. RCW?

1070 ↗

(On Diff #207748)

Current style would be ReadMBB, though I'd be tempted to call this LoopMBB

1071 ↗

(On Diff #207748)

Current style would be SinkMBB though I'd be temped to call this DoneMBB

1072 ↗

(On Diff #207748)

Should be DL

1082 ↗

(On Diff #207748)

When my select custom insertion was being reviewed, the consensus was that reassigning the BB pointer made the logic needlessly hard to follow for little benefit. I suggest just referring to LoopMBB directly instead.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1060 ↗

(On Diff #207748)

Given we you have to list this as CSRRS, I'd phrase this as "we can directly access the 64-bit 'cycle' CSR."

1063 ↗

(On Diff #207748)

"to the suggested loop reading the 'cycle' and 'cycleh' CSRs"?

1066 ↗

(On Diff #207748)

Given opcodestr+argstr should't ever be emitted, I think it makes sense to just remove those strings.

[simoncook]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

843 ↗

(On Diff #207748)

Is it worth having an assertion similar to ISD::SRL below that will trigger when this custom lower is called for rv64 subtargets?

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1062 ↗

(On Diff #207748)

This should have a comment, like on line 611 which notes 0xc00 is a CSR address rather than a random constant.

(I also think it would be nice to have patterns select named InstAliases to use the rdcycle instruction alias directly, but it seems TableGen doesn't support doing such a thing)

[lenary]

Address Review Feedback

• Updated lots of comments
• Added some asserts to trigger if we try to introduce or lower the custom pseudo-instruction on rv64.
• Clarify some magic numbers

Potentially this should wait for D64139, so we can use the named
constants that PR introduces.

[lenary]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1062 ↗

(On Diff #207748)

I'm going to wait for D64139 to land, and then use the constants introduced there.

[asb]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1062 ↗

(On Diff #207748)

Why not just rebase this patch on top of D64139?

[lenary]

Incorporate changes based on D64139, in order to avoid magic numbers.

[asb]

LGTM, thanks!

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

183 ↗

(On Diff #207748)

New nit: reviewing this again, does this comment actually add value?

I'd actually consider a comment like:
`// TODO: On M-mode only targets, the cycle[h] CSR may not be present. Unfortunately this can't be determined just from the ISA naming string."

[lenary]

• Update Comment

llvm/trunk/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

if (Op1.getOpcode() == ISD::Constant &&
             CurDAG->getTargetConstant(ShAmt, SDLoc(Node), XLenVT);
         CurDAG->SelectNodeTo(Node, RISCV::SRLIW, XLenVT, Op0.getOperand(0),
                              ShAmtVal);
         return;
       }
     }
     break;
   }
+  case RISCVISD::READ_CYCLE_WIDE:
+    assert(!Subtarget->is64Bit() && "READ_CYCLE_WIDE is only used on riscv32");
+
+    ReplaceNode(Node, CurDAG->getMachineNode(RISCV::ReadCycleWide, DL, MVT::i32,
+                                             MVT::i32, MVT::Other,
+                                             Node->getOperand(0)));
+    return;
   }
 
   // Select the default instruction.
   SelectCode(Node);
 }
 
 bool RISCVDAGToDAGISel::SelectInlineAsmMemoryOperand(
     const SDValue &Op, unsigned ConstraintID, std::vector<SDValue> &OutOps) {

llvm/trunk/lib/Target/RISCV/RISCVISelLowering.h

enum NodeType : unsigned {
   DIVUW,
   REMUW,
   // FPR32<->GPR transfer operations for RV64. Needed as an i32<->f32 bitcast
   // 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
+  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
 };
 }
 
 class RISCVTargetLowering : public TargetLowering {
   const RISCVSubtarget &Subtarget;
 
 public:
   explicit RISCVTargetLowering(const TargetMachine &TM,

llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp

RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
   }
 
   setOperationAction(ISD::GlobalAddress, XLenVT, Custom);
   setOperationAction(ISD::BlockAddress, XLenVT, Custom);
   setOperationAction(ISD::ConstantPool, XLenVT, Custom);
 
   setOperationAction(ISD::GlobalTLSAddress, XLenVT, Custom);
 
+  // TODO: On M-mode only targets, the cycle[h] CSR may not be present.
+  // Unfortunately this can't be determined just from the ISA naming string.
+  setOperationAction(ISD::READCYCLECOUNTER, MVT::i64,
+                     Subtarget.is64Bit() ? Legal : Custom);
+
   if (Subtarget.hasStdExtA()) {
     setMaxAtomicSizeInBitsSupported(Subtarget.getXLen());
     setMinCmpXchgSizeInBits(32);
   } else {
     setMaxAtomicSizeInBitsSupported(0);
   }
 
   setBooleanContents(ZeroOrOneBooleanContent);

 
 void RISCVTargetLowering::ReplaceNodeResults(SDNode *N,
                                              SmallVectorImpl<SDValue> &Results,
                                              SelectionDAG &DAG) const {
   SDLoc DL(N);
   switch (N->getOpcode()) {
   default:
     llvm_unreachable("Don't know how to custom type legalize this operation!");
+  case ISD::READCYCLECOUNTER: {
+    assert(!Subtarget.is64Bit() &&
+           "READCYCLECOUNTER only has custom type legalization on riscv32");
+
+    SDVTList VTs = DAG.getVTList(MVT::i32, MVT::i32, MVT::Other);
+    SDValue RCW =
+        DAG.getNode(RISCVISD::READ_CYCLE_WIDE, DL, VTs, N->getOperand(0));
+
+    Results.push_back(RCW);
+    Results.push_back(RCW.getValue(1));
+    Results.push_back(RCW.getValue(2));
+    break;
+  }
   case ISD::SHL:
   case ISD::SRA:
   case ISD::SRL:
     assert(N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
            "Unexpected custom legalisation");
     if (N->getOperand(1).getOpcode() == ISD::Constant)
       return;
     Results.push_back(customLegalizeToWOp(N, DAG));
case RISCVISD::REMUW:
     // more precise answer could be calculated for SRAW depending on known
     // bits in the shift amount.
     return 33;
   }
 
   return 1;
 }
 
+MachineBasicBlock *emitReadCycleWidePseudo(MachineInstr &MI,
+                                           MachineBasicBlock *BB) {
+  assert(MI.getOpcode() == RISCV::ReadCycleWide && "Unexpected instruction");
+
+  // To read the 64-bit cycle CSR on a 32-bit target, we read the two halves.
+  // Should the count have wrapped while it was being read, we need to try
+  // again.
+  // ...
+  // read:
+  // rdcycleh x3 # load high word of cycle
+  // rdcycle  x2 # load low word of cycle
+  // rdcycleh x4 # load high word of cycle
+  // bne x3, x4, read # check if high word reads match, otherwise try again
+  // ...
+
+  MachineFunction &MF = *BB->getParent();
+  const BasicBlock *LLVM_BB = BB->getBasicBlock();
+  MachineFunction::iterator It = ++BB->getIterator();
+
+  MachineBasicBlock *LoopMBB = MF.CreateMachineBasicBlock(LLVM_BB);
+  MF.insert(It, LoopMBB);
+
+  MachineBasicBlock *DoneMBB = MF.CreateMachineBasicBlock(LLVM_BB);
+  MF.insert(It, DoneMBB);
+
+  // Transfer the remainder of BB and its successor edges to DoneMBB.
+  DoneMBB->splice(DoneMBB->begin(), BB,
+                  std::next(MachineBasicBlock::iterator(MI)), BB->end());
+  DoneMBB->transferSuccessorsAndUpdatePHIs(BB);
+
+  BB->addSuccessor(LoopMBB);
+
+  MachineRegisterInfo &RegInfo = MF.getRegInfo();
+  unsigned ReadAgainReg = RegInfo.createVirtualRegister(&RISCV::GPRRegClass);
+  unsigned LoReg = MI.getOperand(0).getReg();
+  unsigned HiReg = MI.getOperand(1).getReg();
+  DebugLoc DL = MI.getDebugLoc();
+
+  const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
+  BuildMI(LoopMBB, DL, TII->get(RISCV::CSRRS), HiReg)
+      .addImm(RISCVSysReg::lookupSysRegByName("CYCLEH")->Encoding)
+      .addReg(RISCV::X0);
+  BuildMI(LoopMBB, DL, TII->get(RISCV::CSRRS), LoReg)
+      .addImm(RISCVSysReg::lookupSysRegByName("CYCLE")->Encoding)
+      .addReg(RISCV::X0);
+  BuildMI(LoopMBB, DL, TII->get(RISCV::CSRRS), ReadAgainReg)
+      .addImm(RISCVSysReg::lookupSysRegByName("CYCLEH")->Encoding)
+      .addReg(RISCV::X0);
+
+  BuildMI(LoopMBB, DL, TII->get(RISCV::BNE))
+      .addReg(HiReg)
+      .addReg(ReadAgainReg)
+      .addMBB(LoopMBB);
+
+  LoopMBB->addSuccessor(LoopMBB);
+  LoopMBB->addSuccessor(DoneMBB);
+
+  MI.eraseFromParent();
+
+  return DoneMBB;
+}
+
 static MachineBasicBlock *emitSplitF64Pseudo(MachineInstr &MI,
                                              MachineBasicBlock *BB) {
   assert(MI.getOpcode() == RISCV::SplitF64Pseudo && "Unexpected instruction");
 
   MachineFunction &MF = *BB->getParent();
   DebugLoc DL = MI.getDebugLoc();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
   const TargetRegisterInfo *RI = MF.getSubtarget().getRegisterInfo();

 }
 
 MachineBasicBlock *
 RISCVTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
                                                  MachineBasicBlock *BB) const {
   switch (MI.getOpcode()) {
   default:
     llvm_unreachable("Unexpected instr type to insert");
+  case RISCV::ReadCycleWide:
+    assert(!Subtarget.is64Bit() &&
+           "ReadCycleWrite is only to be used on riscv32");
+    return emitReadCycleWidePseudo(MI, BB);
   case RISCV::Select_GPR_Using_CC_GPR:
   case RISCV::Select_FPR32_Using_CC_GPR:
   case RISCV::Select_FPR64_Using_CC_GPR:
     return emitSelectPseudo(MI, BB);
   case RISCV::BuildPairF64Pseudo:
     return emitBuildPairF64Pseudo(MI, BB);
   case RISCV::SplitF64Pseudo:
     return emitSplitF64Pseudo(MI, BB);
const char *RISCVTargetLowering::getTargetNodeName(unsigned Opcode) const {
   case RISCVISD::DIVUW:
     return "RISCVISD::DIVUW";
   case RISCVISD::REMUW:
     return "RISCVISD::REMUW";
   case RISCVISD::FMV_W_X_RV64:
     return "RISCVISD::FMV_W_X_RV64";
   case RISCVISD::FMV_X_ANYEXTW_RV64:
     return "RISCVISD::FMV_X_ANYEXTW_RV64";
+  case RISCVISD::READ_CYCLE_WIDE:
+    return "RISCVISD::READ_CYCLE_WIDE";
   }
   return nullptr;
 }
 
 std::pair<unsigned, const TargetRegisterClass *>
 RISCVTargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
                                                   StringRef Constraint,
                                                   MVT VT) const {

llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td

 defm : LdPat<load, LD>;
 
 /// Stores
 
 defm : StPat<truncstorei32, SW, GPR>;
 defm : StPat<store, SD, GPR>;
 } // Predicates = [IsRV64]
 
+/// readcyclecounter
+// On RV64, we can directly read the 64-bit "cycle" CSR.
+let Predicates = [IsRV64] in
+def : Pat<(readcyclecounter), (CSRRS CYCLE.Encoding, X0)>;
+// On RV32, ReadCycleWide will be expanded to the suggested loop reading both
+// halves of the 64-bit "cycle" CSR.
+let Predicates = [IsRV32], usesCustomInserter = 1, hasSideEffects = 0,
+mayLoad = 0, mayStore = 0, hasNoSchedulingInfo = 1 in
+def ReadCycleWide : Pseudo<(outs GPR:$lo, GPR:$hi), (ins), [], "", "">;
+
 //===----------------------------------------------------------------------===//
 // Standard extensions
 //===----------------------------------------------------------------------===//
 
 include "RISCVInstrInfoM.td"
 include "RISCVInstrInfoA.td"
 include "RISCVInstrInfoF.td"
 include "RISCVInstrInfoD.td"
 include "RISCVInstrInfoC.td"

llvm/trunk/test/CodeGen/RISCV/readcyclecounter.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32I %s
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64I %s
+
+; Verify that we lower @llvm.readcyclecounter() correctly.
+
+declare i64 @llvm.readcyclecounter()
+
+define i64 @test_builtin_readcyclecounter() nounwind {
+; RV32I-LABEL: test_builtin_readcyclecounter:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:  .LBB0_1: # =>This Inner Loop Header: Depth=1
+; RV32I-NEXT:    rdcycleh a1
+; RV32I-NEXT:    rdcycle a0
+; RV32I-NEXT:    rdcycleh a2
+; RV32I-NEXT:    bne a1, a2, .LBB0_1
+; RV32I-NEXT:  # %bb.2:
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: test_builtin_readcyclecounter:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    rdcycle a0
+; RV64I-NEXT:    ret
+  %1 = tail call i64 @llvm.readcyclecounter()
+  ret i64 %1
+}