[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:168
+    case RISCV::JALR:
+      return Inst.getOperand(0).getReg() == RISCV::X1;
+    }
----------------
pcwang-thead wrote:
> What about `call t0, OUTLINED_FUNCTION` where register is `RISCV::X5`?
It's an interesting question whether we should handle outlined function calls here. I'd say it makes sense to do so but I would like to hear some more opinions about this.

We could also consider returning true for all JAL/JALR instructions where RD!=X0. The reasoning being that if the return address is stored anywhere, it's probably a function call.


================
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;
----------------
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.


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

https://reviews.llvm.org/D146438