[PATCH] AIX struct alignment (PR 99557)

[David Edelsohn]

On Wed, Mar 24, 2021 at 11:30 AM Iain Sandoe <idsandoe@googlemail.com> wrote:
>
> David Edelsohn via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> > On Wed, Mar 24, 2021 at 3:51 AM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> >> On Wed, Mar 24, 2021 at 3:03 AM David Edelsohn <dje.gcc@gmail.com> wrote:
> >>> On Mon, Mar 22, 2021 at 4:10 AM Richard Biener
> >>> <richard.guenther@gmail.com> wrote:
> >>>
> >>>> Oh, and for a type like
> >>>>
> >>>> struct { struct { struct { ... { double x; } } } } } };
> >>>>
> >>>> the layout now looks quadratic in work (each field layout will look at
> >>>> the nest rooted at it
> >>>> up to the bottom).  It looks to me as we require(?) the field types to
> >>>> be laid out and thus
> >>>> at least an early out checking the type alignment to be >= 64 can work?
> >>>
> >>> rs6000_special_round_type_align and rs6000_special_adjust_field_align
> >>> both can have early exits to handle some easy cases.  Thanks for
> >>> pointing that out.
> >>>
> >>> struct A { struct { struct { ... { double x; } } } };
> >>> struct B { struct A; struct A; struct A; struct A; ...; };
> >>>
> >>> is a particularly ugly situation.
> >>>
> >>> When I originally had implemented this in GCC, the recursive nature of
> >>> the requirement was not clear. Changing the alignment for a type
> >>> (struct) in two different contexts (bare versus member) is bizarre,
> >>> but that is what IBM XL compilers implement.
> >>>
> >>> If this becomes a time-sink for for in real use cases, one could
> >>> create a side cache of the type with the previously calculated
> >>> alignment value.  Or are there some preferred, available flag bit in
> >>> the tree that can record that the type alignment has been checked and
> >>> either use TYPE_ALIGN or use 32?  Maybe protected_flag or
> >>> side_effects_flag or nothrow_flag?
> >>
> >> I think type alignment is finalized once a type is laid out which means
> >> checking COMPLETE_TYPE_P (type)  (see layout_type()s early out).
> >
> > Yes, this primarily is a problem for fields, not types.
> >
> >> But then to lay out B we still need, for each field of type A, look
> >> recursively at the first "real" member and check its field alignment?
> >> While we know that A is laid out, it's alignment as-a-field is still
> >> unknown and is not cached anywhere, right?
> >
> > Correct.  The alignment of the bare type is set, but there is no
> > separate information of its type (alignment) as a field.
> >
> >> So while early outs (as I suggested using some bounds on the types
> >> alignment) are possible the worst case will still be present, and indeed,
> >> caching the alignment as-a-field somewhere is the only way to "fix"
> >> this :/  (but I also guess it likely doesn't matter in practice ...)
> >
> > The record alignment can exit if the proposed alignment is >=64 and
> > the field alignment can exit if the proposed alignment is <=32.
> >
> > I guess that I also could test if the record or field type mode is
> > DFmode, DCmode or BLKmode, but most are BLKmode and I don't know if
> > that catches many more cases.
> >
> >> If this particular case is always overriding field alignment with a
> >> special
> >> value then a single bit would be enough to do this and I guess a
> >> (new?) target hook called at layout_type time to compute such properties
> >> would be OK (to avoid requiring another bit to see whether the bit was
> >> already computed).  There's also the possibility to use a target specific
> >> attribute to store such information.
> >
> > How would the new target hook know if the value already was computed?
> >
> >> I guess doing some early outs should avoid most real-world slowdowns.
> >> Btw, does XLC "behave" with the problematic case?
> >
> > XL produces the correct result.  I haven't specifically tested for
> > quadratic behavior.
> >
> > The new LLVM support for AIX adds some additional members to the
> > common part of the LLVM class that describes alignment layout to
> > distinguish between the different types of alignment.  The information
> > is recorded once and not recomputed.
> >
> > I have been bootstrapping with variants of the patch for over a week
> > and haven't noticed any particular change in bootstrap time, either
> > before or after the early exits.
> >
> > One possibility is to commit the current patch and see if anyone
> > complains about compile time performance.
>
> It seems that this might be converging with what Darwin has to do - except
> Darwin has to deal with long long as well as double.
>
> Darwin also has to deal with user-mandated (attribute-wise) alignment or
> packed cases;  not required for AIX?

Hi, Ian

Yes, the AIX adjustment to types is looking more like Darwin.  I
already had added support for packed.

One difference and where Darwin seems to make more sense, is that it
didn't revert the alignment of structs/classes when used as a field.

Darwin uses AGGREGATE_TYPE_P, which includes ARRAY_TYPE in addition to
RECORD_TYPE.  Do you understand the interaction between that and
stripping arrays?  What is the implication of accepting ARRAY_TYPE
when walking the fields?

Thanks, David