[PATCH] D146438: [RISCV][MC] Refine MCInstrAnalysis based on registers used

[Job Noorman via Phabricator via llvm-commits]

jobnoorman added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp:180
+    case RISCV::JALR:
+      return Inst.getOperand(0).getReg() == RISCV::X0 &&
+             Inst.getOperand(1).getReg() == RISCV::X1;
----------------
asb wrote:
> jobnoorman wrote:
> > pcwang-thead wrote:
> > > What about `jalr x0, x5` which is used in outlined function?
> > If we decide to do the same for  `isCall`, this could make sense for consistency. However, I'm a bit afraid that many indirect branches that happen to use X5 will be incorrectly recognized as returns.
> > 
> > Since both `isCall` and `isReturn` can only be estimates on RISC-V given just a single `MCInst`, the question is whether we want to over- or underestimate. I personally lean towards underestimating since returns are, in general, easy to detect on most targets. So giving a lot of false positives here might trip-up tools using this API.
> I'd lean towards being conservative in isReturn, and if a user needs to know if it might be a return, a new `maybeReturn` or similar method can be introduced (and implemented in terms of `isReturn() || some_other_logic` of course).
That makes sense! Any thoughts on implementing `isCall` as "JAL/JALR and RD!=X0"?


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

https://reviews.llvm.org/D146438