[PATCH] D158623: [RISCV] Reorder the stack frame objects.

[lcvon via Phabricator via llvm-commits]

lcvon007 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:575
+
+    // If the two densities are equal, prioritize highest alignment
+    // objects. This allows for similar alignment objects
----------------
reames wrote:
> I think you're missing something really significant here.  If we start with
> 
> ```
> i32, align 4
> i8, align 1
> ```
> 
> And we reorder to:
> 
> ```
> i8, align 1
> i32, align 4
> ```
> 
> There's an extra three bytes of padding added between the objects.  This lengthens the total offset computation, and may push the second object out of the base+offset addressable range.  With a badly chosen set of objects, you can also end up growing the stack very significantly due to all the extra padding.
> 
> I would suggest that you start by restricting yourself to reordering objects of the same alignment.  (i.e. sort only within sets of object with equal alignment)
I adjust the algorithm to sort the objects with same alignment size only(although I know it's not the best order).
I split the following frame objects into four groups rather than three groups,
alignment size of each objects: 1B 4B 4B 1B 4B
group0: 1B
group1: 4B 4B
group2: 1B
group3: 4B
and it will make sure that no extra padding is added.

we may get the smallest frame size if we sort the whole frame objects first, but different groups will have its order first, so it's hard to adjust object in group A before object in group B, or inversely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158623