[PATCH] D93750: [RISCV] Frame handling for RISC-V V extension.

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineFrameInfo.h:226
+  /// vector objects have been allocated yet.
+  bool HasScalableVectorObjects = false;
+
----------------
How come SVE doesn't need this already?


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:280
+  // We do not want the generic alignment handling in
+  // PEI::calculateFrameObjectOffsets(). We store the pointers of RVV objects
+  // in the frame, instead of storing RVV objects themselves. That is why we
----------------
Hm, looking at the stack-realignment.ll diff, were we double-counting before? If so it'd be worth splitting out that bit of this revision into its own smaller one.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:496
     Register Reg = Entry.getReg();
+    // We don't have sensible DWARF for VRs yet
+    if (RISCV::VRRegClass.contains(Reg))
----------------
:( is there progress being made to specify it?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:249
   bit HasDummyMask = 0;
+  bit IsRVVLoadStore = 0;
 }
----------------
Can you not just do `isa<>` to check it's the right class and then look at mayLoad/mayStore?


================
Comment at: llvm/test/CodeGen/RISCV/rvv/allocate-lmul-2-4-8.ll:5
+
+define void @lmul1() nounwind optnone noinline {
+; CHECK-LABEL: lmul1:
----------------
Only nounwind is needed for these I believe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93750