On Wed, Mar 24, 2021 at 4:23 PM David Edelsohn <dje....@gmail.com> wrote:
>
> On Wed, Mar 24, 2021 at 3:51 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Wed, Mar 24, 2021 at 3:03 AM David Edelsohn <dje....@gmail.com> wrote:
> > >
> > > On Mon, Mar 22, 2021 at 4:10 AM Richard Biener
> > > <richard.guent...@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?
Good question. I suppose we'd redundantly compute it when re-layouting.
I see we're "wasting" 6 bits for warn_if_not_align alongside TYPE_ALIGN
and thus (given another spare 16 bits) "wasting" another 6 bits for
TYPE_ALIGN_AS_FIELD (or _IN_STRUCT?) might be possible as well.
That said, if actual problems arise.
Richard.
> >
> > 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.
>
> Thanks, David
