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

[Wang Pengcheng via Phabricator via llvm-commits]

pcwang-thead 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";
----------------
craig.topper wrote:
> 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?
I'm not for sure. Here is an example in `llvm/lib/Target/AArch64/AArch64InstrInfo.td`:
```
let isCall = 1, Defs = [LR, X0, X1], hasSideEffects = 1, Size = 16,
    isCodeGenOnly = 1 in
def TLSDESC_CALLSEQ
    : Pseudo<(outs), (ins i64imm:$sym),
             [(AArch64tlsdesc_callseq tglobaltlsaddr:$sym)]>,
      Sched<[WriteI, WriteLD, WriteI, WriteBrReg]>;
```
Defs are 3 registers ([LR, X0, X1]) and there is no output register, while there are 4 writes.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123578