[PATCH] D70821: [SystemZ] Implement the packed stack layout

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 7 02:53:38 PST 2019


uweigand added a comment.

> It seems that PEI::calculateFrameObjectOffsets() is taking the Local Area Offset into account, while in fact the very "similar" MFFrame.estimateStackSize() in fact does not (but probably should).

Ah, I see.   Yes, the current MFFrame.estimateStackSize looks incorrect -- it does not take account the Local Area Offset, but it still accounts for whatever fixed objects happen to be placed inside that offset; that's inconsistent.  Either it should always fully include the Local Area Offset in the stack size, or else it should always exclude it, but then it must also exclude objects that happen to be placed there.

It seems this problem wasn't noticed so far since MFFrame.estimateStackSize is never used by common code, it is simply a helper routine that can be used by targets.  And all targets that use it (except SystemZ!) just happen to use a zero Local Area Offset, so the problem doesn't occur.  And even we didn't use the helper until recently; I didn't notice that problem when I added it.


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

https://reviews.llvm.org/D70821





More information about the llvm-commits mailing list