[PATCH] D113967: [RISCV] Reverse the order of loading/storing callee-saved registers.

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In D113967#3135737 <https://reviews.llvm.org/D113967#3135737>, @jrtc27 wrote:

> Though, I'm unconvinced about whether the hazard in the epilogue matters in practice. In a simple in-order micoarchitecture if restoring ra stalls then it doesn't matter where that is, you can't execute out-of-order. In a superscalar core you'll just speculate past the ret anyway, which should predict accurately for anything with a half-decent RAS, though I guess you'll have unresolved speculation for longer that might be an issue. Is there a particular microarchitecture you've measured an appreciable difference for this on?

The target is probably cores using a microarchitecture similar to Rocket. It has a "non-blocking D-cache" where the integer pipeline won't stall upon a cache miss until the register value is actually read (provided there are sufficient MSHRs).



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:440
   // directives.
-  for (const auto &Entry : CSI) {
+  for (const auto &Entry : reverse(CSI)) {
     int FrameIdx = Entry.getFrameIdx();
----------------
Worth adding a comment to explain why list is being iterated in reverse order.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1045
   const auto &NonLibcallCSI = getNonLibcallCSI(*MF, CSI);
-  for (auto &CS : NonLibcallCSI) {
+  for (auto &CS : reverse(NonLibcallCSI)) {
     // Insert the spill to the stack frame.
----------------
Worth adding a comment to explain why list is being iterated in reverse order.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1071
   const auto &NonLibcallCSI = getNonLibcallCSI(*MF, CSI);
-  for (auto &CS : reverse(NonLibcallCSI)) {
+  for (auto &CS : NonLibcallCSI) {
     Register Reg = CS.getReg();
----------------
I think the comment above needs updating now, and again it would be worth explaining the desired iteration order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113967