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?
Iain
