[PATCH] D157373: [RISCV] add a compress optimization for stack inst.

[lcvon via Phabricator via llvm-commits]

lcvon007 added a comment.

> Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?

As you see in the first version of implementation, it will add extra instructions(because the second SP amount may be too larger to use more instructions to build the immediate) if we don't add this condition, and the performance may regress in some cases, and it's better to remove this condition if we only want to optimize the codesize.



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1324
+      // Avoid increasing extra instructions when ld/st can be compressed.
+      if ((CSI.size() <= RVCompressLen) && (StackSize <= RVCompressLen + 2048 ||
+                                            StackSize > 2048 * 3 - StackAlign))
----------------
wangpc wrote:
> Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?
> Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?

As you see in the first version of implementation, it will add extra instructions(because the second SP amount may be too larger to use more instructions to build the immediate) if we don't add this condition, and the performance may regress in some cases, and it's better to remove this condition if we only want to optimize the codesize.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:1324
+      // Avoid increasing extra instructions when ld/st can be compressed.
+      if ((CSI.size() <= RVCompressLen) && (StackSize <= RVCompressLen + 2048 ||
+                                            StackSize > 2048 * 3 - StackAlign))
----------------
lcvon007 wrote:
> wangpc wrote:
> > Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?
> > Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?
> 
> As you see in the first version of implementation, it will add extra instructions(because the second SP amount may be too larger to use more instructions to build the immediate) if we don't add this condition, and the performance may regress in some cases, and it's better to remove this condition if we only want to optimize the codesize.
> Do we really need this condition `(StackSize <= RVCompressLen + 2048 || StackSize > 2048 * 3 - StackAlign)`?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157373