[PATCH] D66210: [RISCV] Enable the machine outliner for RISC-V

[Stefan O'Rear via Phabricator via llvm-commits]

sorear added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:481
+  RS.enterBasicBlock(MBB);
+  return !RS.isRegUsed(RISCV::X5);
+}
----------------
luismarques wrote:
> lewis-revill wrote:
> > luismarques wrote:
> > > If we are only going to support one possible register for now, shouldn't it be the one least likely to already be in use? Wouldn't that be t6 (x31)?
> > That's a sensible suggestion, I was using t0 to match how the save/restore libcalls behave so I presumed there was a good reason for using t0. Don't we also need to think about RV32E here though?
> > 
> > 
> Good point about RV32E. I guess you can either always use `t2` or check the target and use `t6` when available, falling back to `t2` otherwise. If that's not trivial it might be worth checking how hard it would be to dynamically choose the register, like AArch64 does IIRC.
`ra` and `t0` (`x1` and `x5`) have [special functionality in implementations with a return-address stack](https://content.riscv.org/wp-content/uploads/2019/06/riscv-spec.pdf#page=38); `jr t6` will be treated as a general indirect branch, not a return, and is much more likely to mispredict.  So this should probably prefer `t0` whenever possible.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:548
+  // RISCV::PseudoCALL = 8 bytes.
+  unsigned CallOverhead = 8;
+  for (auto &C : RepeatedSequenceLocs)
----------------
Tangentially related, but in many of the cases where outlining makes sense, it would also make sense to generate 4-byte `jal t0, label` instructions; lld and ld.bfd would need to be taught to generate thunks for out of range `jal` (the Go linker already supports them).


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:552
+
+  // RISCV::PseudoRET = 4 bytes.
+  unsigned FrameOverhead = 4;
----------------
Note that `jr <reg>` is a compressible instruction regardless of the register used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66210