jrtc27 added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:38
+  // Do not save RA to the SCS if it's not saved to the regular stack,
+  // i.e. RA is not at risk of being to overwritten.
+  std::vector<CalleeSavedInfo> &CSI = MF.getFrameInfo().getCalleeSavedInfo();
----------------



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:60
+  // sw   ra, 0(s2)
+  // addi s2, s2, 4
+  BuildMI(MBB, MI, DL, TII->get(IsRV64 ? RISCV::SD : RISCV::SW))
----------------
zzheng wrote:
> jrtc27 wrote:
> > Is it intended that the shadow call stack grows *up* unlike the normal 
> > stack?
> No. Which direction the SCS grows on is trivial.
> 
> The memory area hosting SCS is independent of the regular stack; and it's 
> provided by the runtime.
> mmap/malloc returns the low address of newly mapped/allocated area. Making 
> the SCS growing down requires the runtime to return upper bound of the SCS. 
> On AArch64, the SCS grows up as well.
Ok. Wasn't saying there was anything wrong with it, was just something that 
jumped out at me. Having it grow up makes more sense (the only real advantage 
to the normal stack growing down these days is that doing aligned allocations 
is slightly cheaper).


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:79-80
+
+  // Do not restore RA from SCS if it's not saved to regular stack, i.e.
+  // RA is not subject to overwritten.
+  std::vector<CalleeSavedInfo> &CSI = MF.getFrameInfo().getCalleeSavedInfo();
----------------
jrtc27 wrote:
> No need to repeat ourselves.
This still holds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to