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

[Wang Pengcheng via Phabricator via llvm-commits]

wangpc added inline comments.


================
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:
> lcvon007 wrote:
> > 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)`?
> > 
> > 
> > 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.
If so, then I think we can loose the condition when optimizing for size? What do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157373