[PATCH] D123496: Add Stackmap support for RISC-V

[Luís Marques via Phabricator via llvm-commits]

luismarques added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:160
+  if (CallTarget) {
+    assert((CallTarget & 0xFFFFFFFFFFFF) == CallTarget &&
+           "High 16 bits of call target should be zero.");
----------------
Nit: put some `'` digit separators to make it easier to read the constant. E.g. `0x0xFFFF'FFFF'FFFF`. (Or use some higher-level function from `MathExtras.h` if there is one).


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:166-193
+    EmitToStreamer(OutStreamer, MCInstBuilder(RISCV::ORI)
+                                    .addReg(RISCV::X1)
+                                    .addReg(RISCV::X0)
+                                    .addImm((CallTarget >> 36) & 0xFFF));
+    EmitToStreamer(OutStreamer, MCInstBuilder(RISCV::SLLI)
+                                    .addReg(RISCV::X1)
+                                    .addReg(RISCV::X1)
----------------
Can't we do better than this for materializing the address? If the number of instructions doesn't have to be fixed you can even use `generateInstSeq` from `RISCVMatInt.h` to generate a more optimal instruction sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123496