[PATCH] D62190: [RISCV] Allow shrink wrapping for RISC-V

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

luismarques accepted this revision.
luismarques added a comment.
This revision is now accepted and ready to land.

Thanks for the patch Lewis. Overall it LGTM, and it would be great to merge this in time for the LLVM 10.0 release.

The patch would benefit from having more extensive tests, although those could be provided by a follow-up patch (but please do follow up). For instance, the current tests don't exercise the `if (!MBB.empty())` `else` condition. It would also be great to include tests that illustrate the impact on debug instructions (even if that has been an area where testing has been limited in the past). See for instance my inline comment for a possible issue regarding correctly updating the debug locations.



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:261
+      MBBI = MBB.getLastNonDebugInstr();
+    DebugLoc DL = MBBI->getDebugLoc();
+
----------------
This declaration shadows the `DL` above; I assume you meant to update the previously declared `DL`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62190