[PATCH] D123496: [RISCV] Add Stackmap/Statepoint/Patchpoint support without targets

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

Why do you have both stackmap.ll and rv64-stackmap.ll? And why does stackmap-frame-setup.ll not have an rv64- prefix like the rest (once that duplication is resolved)?



================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:101
+  SM.recordStackMap(*MILabel, MI);
+  assert(NumNOPBytes % 4 == 0 && "Invalid number of NOP bytes requested!");
+
----------------
Zeavee wrote:
> jrtc27 wrote:
> > Is it still invalid with RVC? You can have 2-byte NOPs there...
> I indeed missed the C_NOP. Should we add one when NumNOPBytes % 4 == 2 then, because emitNops only emits 4 bytes NOPs?
I don't know, it depends what you want stackmap code to do, which is something I'm not familiar with. All I know is that, without additional context, requesting 18 bytes of NOPs is a perfectly reasonable thing to do on RISC-V, just as requesting 7 bytes is reasonable on x86 due to it having 1-byte NOPs (0x90). I'll note that @reames already pointed out the discrepancy between emitNops and getNumPatchBytes in terms of bytes vs instructions below.

Also, AsmPrinter::emitNops emits whatever TII->getNop() returns, which is C_NOP on RISC-V if RVC is enabled, so you currently have a bug that this is half the number of bytes being requested in that case.


================
Comment at: llvm/test/CodeGen/RISCV/rv64-patchpoint.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -debug-entry-values -enable-misched=0                             < %s | FileCheck %s
+
----------------
You have a bunch of spaces like this left in various files


================
Comment at: llvm/test/CodeGen/RISCV/rv64-stackmap.ll:3
+;
+; Note: Print verbose stackmaps using -debug-only=stackmaps.
+
----------------
Why's this in a test? This is documentation for how to use llc...


================
Comment at: llvm/test/CodeGen/RISCV/stackmap.ll:3
+;
+; Note: Print verbose stackmaps using -debug-only=stackmaps.
+
----------------
Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123496