[PATCH] D123578: [RISCV] Add sched to pseudo function call instructions

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a subscriber: andreadb.
craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1282
     mayStore = 0, mayLoad = 0 in
-def PseudoCALLReg : Pseudo<(outs GPR:$rd), (ins call_symbol:$func), []> {
+def PseudoCALLReg : Pseudo<(outs GPR:$rd), (ins call_symbol:$func), []>, Sched<[WriteIALU, WriteJalr, ReadJalr]> {
   let AsmString = "call\t$rd, $func";
----------------
pcwang-thead wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > Don't exceed 80 characters per line.
> > I'm not sure it's correct to have more than one Write scheduling class with only a single output register. Each Write is supposed to correspond to the latency of the register.
> According to comments of `Sched` class in `llvm/include/llvm/Target/TargetSchedule.td`:
> 
> > One SchedWrite type must be listed for each explicit def operand in order. Additional SchedWrite types may optionally be listed for implicit def operands.
> 
> It is legal to add more Writes for implicit operands and I see a lot of definitions like this in other targets. Example in AArch64:
> ```
> def TLSDESC_CALLSEQ
>     : Pseudo<(outs), (ins i64imm:$sym),
>              [(AArch64tlsdesc_callseq tglobaltlsaddr:$sym)]>,
>       Sched<[WriteI, WriteLD, WriteI, WriteBrReg]>;
> ```
> 
> 
> 
My understanding of "implicit def operands" would be physical registers listed in `let Defs = []` syntax.

@andreadb is this the right thing to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123578