[llvm] [RISCV][GlobalISel] Legalize readcyclecounter/readsteadycounter (PR #217535)

[Craig Topper via llvm-commits]

================
@@ -889,6 +895,38 @@ bool RISCVLegalizerInfo::legalizeVAStart(MachineInstr &MI,
   return true;
 }
 
+bool RISCVLegalizerInfo::legalizeReadCounter(
+    MachineInstr &MI, MachineIRBuilder &MIRBuilder) const {
+  assert((MI.getOpcode() == TargetOpcode::G_READCYCLECOUNTER ||
+          MI.getOpcode() == TargetOpcode::G_READSTEADYCOUNTER) &&
+         "Unexpected opcode");
+  assert(!STI.is64Bit() && "READCYCLECOUNTER/READSTEADYCOUNTER only "
+                           "has custom type legalization on riscv32");
+
+  // On RV32 a 64-bit counter CSR must be read as two 32-bit halves. Lower to
+  // the ReadCounterWide target pseudo.
+  bool IsCycle = MI.getOpcode() == TargetOpcode::G_READCYCLECOUNTER;
+  int64_t LoCounter = IsCycle ? RISCVSysReg::cycle : RISCVSysReg::time;
+  int64_t HiCounter = IsCycle ? RISCVSysReg::cycleh : RISCVSysReg::timeh;
+
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
+  auto CreateGPR = [&]() {
+    Register R = MRI.createGenericVirtualRegister(LLT::scalar(32));
+    MRI.setRegClass(R, &RISCV::GPRRegClass);
+    return R;
+  };
+  Register LoReg = CreateGPR();
+  Register HiReg = CreateGPR();
+
+  Register DstReg = MI.getOperand(0).getReg();
+  MIRBuilder.setDebugLoc(MI.getDebugLoc());
+  MIRBuilder.buildInstr(RISCV::ReadCounterWide, {LoReg, HiReg},
----------------
topperc wrote:

I agree that's why the pseudo exists and GISel can directly emit the split.

But from a code maintenance perspective, why do we want two different pieces of code creating the same loop? I don't expect this sequence to change, but if it does we have to update 2 places and might forget one. If SelectionDAG gets fully replaced with GISel we could do the migration then. Until then I'm not sure I see the benefit.

@arsenm @lenary what do you think?

https://github.com/llvm/llvm-project/pull/217535