[PATCH] D79141: [RISCV] Better Split Stack Pointer Adjustment for RVC

[Sam Elliott via Phabricator via llvm-commits]

lenary updated this revision to Diff 300508.
lenary marked 6 inline comments as done.
lenary added a comment.
Herald added a subscriber: NickHung.

Address some comment phrasing issues.

I have not had the time to look at @asb's suggestions around gating the
behaviour of these changes -- I'm planning to hand this patch off to
@luismarques so we can get it finished and landed soon, as I don't expect to
have the time to investigate those issues in the near future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79141

Files:
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.h


Index: llvm/lib/Target/RISCV/RISCVFrameLowering.h
===================================================================
--- llvm/lib/Target/RISCV/RISCVFrameLowering.h
+++ llvm/lib/Target/RISCV/RISCVFrameLowering.h
@@ -58,11 +58,17 @@
                               const TargetRegisterInfo *TRI) const override;
 
   /**
-   * Returns if we want to adjust the SP in two adjustments in the prolog and
-   * epilog, or keep it as just one.
+   * Returns whether the stack pointer (SP) should be adjusted in two
+   * adjustments in the prologue and epilogue ("split"), or only adjusted once.
+   *
+   * Splitting the SP adjustment can result in better code size.
    *
    * The result will be `None` if the SP adjustment should not be split, or an
-   * optional containing the first SP adjustment amount if it should be split.
+   * Optional containing the first adjustment amount if the adjustment should be
+   * split.
+   *
+   * The first SP adjustment will never be more than the function's StackSize,
+   * so that the second SP adjustment is monotinic.
    */
   Optional<uint64_t>
   getFirstSPAdjustmentAmount(const MachineFunction &MF) const;
Index: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -732,31 +732,33 @@
   return MBB.erase(MI);
 }
 
-/**
- * We might like to split the SP adjustment to reduce prologue/epilogue as shown
- * in the following instructions. The reason to do this is twofold: so that the
- * offset for the callee-saved register saves/restores fits into a single
- * function; and to help the SP adjustment amounts fit into two `addi`
- * instructions rather than needing to materialise those immediates with extra
- * instructions.
- *
- *     add     sp,sp,-2032
- *     sw      ra,2028(sp)
- *     sw      s0,2024(sp)
- *     sw      s1,2020(sp)
- *     sw      s3,2012(sp)
- *     sw      s4,2008(sp)
- *     add     sp,sp,-64
- *
- * If this function returns `None`, then we should not split the SP adjustment.
- *
- * If this function returns an Optional containing a value, then the value is
- * the first adjustment for the stack pointer (and the second can be calculated
- * by taking the difference between this and the function's StackSize).
- *
- * The returned value should always be between 0 and the function's stacksize -
- * the intention being that both stack pointer adjustments are monotonic.
- */
+// Splitting the stack pointer (sp) adjustment may reduce the number or size of
+// instructions in the prologue and epilogue, as shown in the following
+// instructions.
+//
+// The reason to do this is twofold: so that the offset for the callee-saved
+// register saves/restores fits into a single (potentially compressed)
+// instruction; and to ensure the SP adjustment amounts fit into one or two
+// `addi` instructions rather than needing to materialise those immediates with
+// extra instructions.
+//
+//     add     sp,sp,-2032
+//     sw      ra,2028(sp)
+//     sw      s0,2024(sp)
+//     sw      s1,2020(sp)
+//     sw      s3,2012(sp)
+//     sw      s4,2008(sp)
+//     add     sp,sp,-64
+//
+// If this function returns `None`, then the sp adjustment should be done in a
+// single step.
+//
+// If this function returns an Optional containing a value, then the value is
+// the first adjustment for the stack pointer (and the second can be calculated
+// by taking the difference between this and the function's StackSize).
+//
+// The returned value should always be between 0 and the function's StackSize -
+// the intention being that both sp adjustments are monotonic.
 Optional<uint64_t> RISCVFrameLowering::getFirstSPAdjustmentAmount(
     const MachineFunction &MF) const {
   const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79141.300508.patch
Type: text/x-patch
Size: 3884 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201024/e2c9d84f/attachment.bin>