[PATCH] D71504: [PowerPC] Enable sret arguments.

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 12:11:57 PST 2019


sfertile marked 4 inline comments as done.
sfertile added a comment.

In D71504#1787912 <https://reviews.llvm.org/D71504#1787912>, @hubert.reinterpretcast wrote:

> In D71504#1787758 <https://reviews.llvm.org/D71504#1787758>, @ZarkoCA wrote:
>
> > I don't know what happens in the case where the front-end doesn't handle some cases correctly but the back-end does.  Is there a danger in enabling this in the back-end knowing we don't get correct IR for complex types for AIX?  Is there an error in the front-end for those before we get to this step?
>
>
> There's no error from the front-end. In terms of continuous delivery, this leads to silently bad code that could lead to incorrect output or crashes at runtime.


I think if the front-end AIX support was further along then a patch like this should be deferred until we fixed (and verified) the IR the front-end generates for sret arguments. With how `clang/CodeGen` is going through the SVR4 targets for AIX its safe to consider any Clang AIX support as extremely experimental right now, and the back-end implementation shouldn't be held up based on Clang support for the target.



================
Comment at: llvm/test/CodeGen/PowerPC/aix-sret-param.ll:35
+; MIR:      stack:
+; MIR-NEXT:  - { id: 0, name: s, type: default, offset: 0, size: 1, alignment: 8,
+
----------------
hubert.reinterpretcast wrote:
> sfertile wrote:
> > sfertile wrote:
> > > hubert.reinterpretcast wrote:
> > > > What is the cause of this alignment value?
> > > Frame object alignment smaller then 8 gets promoted  ... but I'm not exactly sure why 8 as opposed to 4 or 16. I'll figure out where the 8 comes from.
> > When determining the alignment of the frame object we consider 3 different alignments and take the maximum of those. The three alignments are
> > 1) The preferred aggregate alignment which is 8 by default: [[ https://github.com/llvm/llvm-project/blob/2c8e22d25c2091764ccde67812a3f707277052b2/llvm/lib/IR/DataLayout.cpp#L171 | here]]
> > 2) The alignment from the StructLayout object.
> > 3) The alignment on the alloca instruction.
> > 
> > So any alignment less then 8 will get promoted to 8 when creating the frame object.
> > 
> Got it. Thanks for looking into this.
np.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71504





More information about the llvm-commits mailing list