On Wed, Mar 24, 2021 at 11:30 AM Iain Sandoe <idsan...@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.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? > > > >> 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