[PATCH] D55335: [RISCV, WIP] Support assembling @plt symbol operands

Lewis Revill via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 7 13:49:03 PST 2018


lewis-revill added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:115
   const MCExpr *CallExpr =
-      RISCVMCExpr::create(Expr, RISCVMCExpr::VK_RISCV_CALL, Ctx);
+      RISCVMCExpr::create(Expr, Kind, Ctx);
 
----------------
jrtc27 wrote:
> Isn't this just copying `Expr` if it's a `RISCVMCExpr` (assuming `Ctx` ends up being the same context)? If so, we can just re-use `Expr` for that case. But thinking about it further, can we not make sure that `PseudoCALL`/`PseudoTAIL` always get a `RISCVMCExpr`, either with a `VK_RISCV_CALL` or a `VK_RISCV_CALL_PLT`, so everywhere is always explicit about which it wants rather than having an implicit fallback?
I agree, it would be a better way to do it. This confirms your opinion about adding a new operand for call/tail so we can then add `VK_RISCV_CALL`/`VK_RISCV_CALL_PLT` when parsing it. I think what I'd need to do first is to create a patch implementing that, IE adding a `CallSymbol` operand, changing parsing/codegen to attach `VK_RISCV_CALL` to the `RISCVMCExpr`, which `expandFunctionCall` should just emit as is. After I've done that I could make the change in this patch & the codegen patch to incorporate that..


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55335





More information about the llvm-commits mailing list