[PATCH] D64125: [RISCV] Support @llvm.readcyclecounter() Intrinsic

[Simon Cook via Phabricator via llvm-commits]

simoncook added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:843
     llvm_unreachable("Don't know how to custom type legalize this operation!");
+  case ISD::READCYCLECOUNTER: {
+    SDVTList VTs = DAG.getVTList(MVT::i32, MVT::i32, MVT::Other);
----------------
asb wrote:
> Should add an assert that `!Subtarget.is64Bit()`
Is it worth having an assertion similar to `ISD::SRL` below that will trigger when this custom lower is called for rv64 subtargets?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1062
+let Predicates = [IsRV64] in
+def : Pat<(readcyclecounter), (CSRRS 0xC00, X0)>;
+// On RV32, ReadCycleWide is inserted and expanded to the suggested rdcycle[h] loop.
----------------
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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64125/new/

https://reviews.llvm.org/D64125