| Message ID | CAGWvnymKF4TQrdb=cE=E6BnvrZ7NQVJs2JX60OkBbufnf+GF_w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | AIX struct alignment (PR 99557) | expand |
On Mon, Mar 22, 2021 at 3:04 AM David Edelsohn via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The AIX power alignment rules apply the natural alignment of the > "first member" if it is of a floating-point data type (or is an aggregate > whose recursively "first" member or element is such a type). The alignment > associated with these types for subsequent members use an alignment value > where the floating-point data type is considered to have 4-byte alignment. > > GCC had been stripping array type but had not recursively looked > within structs and unions. This also applies to classes and > subclasses and, therefore, becomes more prominent with C++. > > For example, > > struct A { > double x[2]; > int y; > }; > struct B { > int i; > struct A a; > }; > > struct A has double-word alignment when referenced independently, but > word alignment and offset within struct B despite the alignment of > struct A. If struct A were the first member of struct B, struct B > would have double-word alignment. One must search for the innermost > first member to increase the alignment if double and then search for > the innermost first member to reduce the alignment if the TYPE had > double-word alignment solely because the innermost first member was > double. > > This patch recursively looks through the first member to apply the > double-word alignment to the struct / union as a whole and to apply > the word alignment to the struct or union as a member within a struct > or union. > > This is an ABI change for GCCon AIX, but GCC on AIX had not correctly > implemented the AIX ABI and had not been compatible with the IBM XL > compiler. > > If anyone can double-check that the patch walks the fields correctly > and handles the error conditions correctly, it would be appreciated. > > Bootstrapped on powerpc-ibm-aix7.2.3.0. > > Thanks, David > > * config/rs6000/aix.h (ADJUST_FIELD_ALIGN): Call function. > * config/rs6000/rs6000-protos.h: Declare. > * config/rs6000/rs6000.c: Define. > > index 2db50c8007f..7fccb31307b 100644 > --- a/gcc/config/rs6000/aix.h > +++ b/gcc/config/rs6000/aix.h > @@ -223,10 +223,8 @@ > /* This now supports a natural alignment mode. */ > /* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ > #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \ > - ((TARGET_ALIGN_NATURAL == 0 \ > - && (TYPE_MODE (strip_array_types (TYPE)) == DFmode \ > - || TYPE_MODE (strip_array_types (TYPE)) == DCmode)) \ > - ? MIN ((COMPUTED), 32) \ > + (TARGET_ALIGN_NATURAL == 0 \ > + ? rs6000_special_adjust_field_align (TYPE, COMPUTED) > \ > : (COMPUTED)) > > /* AIX increases natural record alignment to doubleword if the first > diff --git a/gcc/config/rs6000/rs6000-protos.h > b/gcc/config/rs6000/rs6000-protos.h > index 203660b0a78..c44fd3d0263 100644 > --- a/gcc/config/rs6000/rs6000-protos.h > +++ b/gcc/config/rs6000/rs6000-protos.h > @@ -227,6 +227,7 @@ address_is_prefixed (rtx addr, > #ifdef TREE_CODE > extern unsigned int rs6000_data_alignment (tree, unsigned int, enum data_align) > ; > extern bool rs6000_special_adjust_field_align_p (tree, unsigned int); > +extern unsigned int rs6000_special_adjust_field_align (tree, unsigned int); > extern unsigned int rs6000_special_round_type_align (tree, unsigned int, > unsigned int); > extern unsigned int darwin_rs6000_special_round_type_align (tree, unsigned int, > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 712dd1c460b..eed51e8d4a2 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -7856,6 +7856,41 @@ rs6000_special_adjust_field_align_p (tree type, unsigned > int computed) > return false; > } > > +/* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ > + > +unsigned int > +rs6000_special_adjust_field_align (tree type, unsigned int computed) > +{ > + /* If RECORD or UNION, recursively find the first field. */ > + while (TREE_CODE (type) == RECORD_TYPE > + || TREE_CODE (type) == UNION_TYPE > + || TREE_CODE (type) == QUAL_UNION_TYPE) RECORD_OR_UNION_TYPE_P (type) > + { > + tree field = TYPE_FIELDS (type); > + > + /* Skip all non field decls */ > + while (field != NULL > + && (TREE_CODE (field) != FIELD_DECL > + || DECL_FIELD_ABI_IGNORED (field))) > + field = DECL_CHAIN (field); > + > + if (field != NULL && field != type) when is the field (a FIELD_DECL) ever pointer equal to the containing record type? Unless you ment to check for sth specific I suggest to drop field != type. > + type = TREE_TYPE (field); > + else > + break; > + } > + > + /* Strip arrays. */ > + while (TREE_CODE (type) == ARRAY_TYPE) > + type = TREE_TYPE (type); > + > + if (type != error_mark_node > + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > + computed = MIN (computed, 32); > + > + return computed; > +} > + > /* AIX increases natural record alignment to doubleword if the first > field is an FP double while the FP fields remain word aligned. */ > @@ -7864,25 +7899,33 @@ rs6000_special_round_type_align (tree type,unsigned int > computed, > unsigned int specified) > { > unsigned int align = MAX (computed, specified); > - tree field = TYPE_FIELDS (type); > > - /* Skip all non field decls */ > - while (field != NULL > - && (TREE_CODE (field) != FIELD_DECL > - || DECL_FIELD_ABI_IGNORED (field))) > - field = DECL_CHAIN (field); > - > - if (field != NULL && field != type) > + /* If RECORD or UNION, recursively find the first field. */ > + while (TREE_CODE (type) == RECORD_TYPE > + || TREE_CODE (type) == UNION_TYPE > + || TREE_CODE (type) == QUAL_UNION_TYPE) same as above > { > - type = TREE_TYPE (field); > - while (TREE_CODE (type) == ARRAY_TYPE) > - type = TREE_TYPE (type); > + tree field = TYPE_FIELDS (type); > + > + /* Skip all non field decls */ > + while (field != NULL > + && (TREE_CODE (field) != FIELD_DECL > + || DECL_FIELD_ABI_IGNORED (field))) > + field = DECL_CHAIN (field); > > - if (type != error_mark_node > - && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > - align = MAX (align, 64); > + if (field != NULL && field != type) > + type = TREE_TYPE (field); > + else > + break; > } > > + while (TREE_CODE (type) == ARRAY_TYPE) > + type = TREE_TYPE (type); > + > + if (type != error_mark_node > + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > + align = MAX (align, 64); it looks like the changes are duplicate and could see some factoring. Richard. > + > + > return align; > }
On Mon, Mar 22, 2021 at 9:06 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Mar 22, 2021 at 3:04 AM David Edelsohn via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > The AIX power alignment rules apply the natural alignment of the > > "first member" if it is of a floating-point data type (or is an aggregate > > whose recursively "first" member or element is such a type). The alignment > > associated with these types for subsequent members use an alignment value > > where the floating-point data type is considered to have 4-byte alignment. > > > > GCC had been stripping array type but had not recursively looked > > within structs and unions. This also applies to classes and > > subclasses and, therefore, becomes more prominent with C++. > > > > For example, > > > > struct A { > > double x[2]; > > int y; > > }; > > struct B { > > int i; > > struct A a; > > }; > > > > struct A has double-word alignment when referenced independently, but > > word alignment and offset within struct B despite the alignment of > > struct A. If struct A were the first member of struct B, struct B > > would have double-word alignment. One must search for the innermost > > first member to increase the alignment if double and then search for > > the innermost first member to reduce the alignment if the TYPE had > > double-word alignment solely because the innermost first member was > > double. > > > > This patch recursively looks through the first member to apply the > > double-word alignment to the struct / union as a whole and to apply > > the word alignment to the struct or union as a member within a struct > > or union. > > > > This is an ABI change for GCCon AIX, but GCC on AIX had not correctly > > implemented the AIX ABI and had not been compatible with the IBM XL > > compiler. > > > > If anyone can double-check that the patch walks the fields correctly > > and handles the error conditions correctly, it would be appreciated. > > > > Bootstrapped on powerpc-ibm-aix7.2.3.0. > > > > Thanks, David > > > > * config/rs6000/aix.h (ADJUST_FIELD_ALIGN): Call function. > > * config/rs6000/rs6000-protos.h: Declare. > > * config/rs6000/rs6000.c: Define. > > > > index 2db50c8007f..7fccb31307b 100644 > > --- a/gcc/config/rs6000/aix.h > > +++ b/gcc/config/rs6000/aix.h > > @@ -223,10 +223,8 @@ > > /* This now supports a natural alignment mode. */ > > /* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ > > #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \ > > - ((TARGET_ALIGN_NATURAL == 0 \ > > - && (TYPE_MODE (strip_array_types (TYPE)) == DFmode \ > > - || TYPE_MODE (strip_array_types (TYPE)) == DCmode)) \ > > - ? MIN ((COMPUTED), 32) \ > > + (TARGET_ALIGN_NATURAL == 0 \ > > + ? rs6000_special_adjust_field_align (TYPE, COMPUTED) > > \ > > : (COMPUTED)) > > > > /* AIX increases natural record alignment to doubleword if the first > > diff --git a/gcc/config/rs6000/rs6000-protos.h > > b/gcc/config/rs6000/rs6000-protos.h > > index 203660b0a78..c44fd3d0263 100644 > > --- a/gcc/config/rs6000/rs6000-protos.h > > +++ b/gcc/config/rs6000/rs6000-protos.h > > @@ -227,6 +227,7 @@ address_is_prefixed (rtx addr, > > #ifdef TREE_CODE > > extern unsigned int rs6000_data_alignment (tree, unsigned int, enum data_align) > > ; > > extern bool rs6000_special_adjust_field_align_p (tree, unsigned int); > > +extern unsigned int rs6000_special_adjust_field_align (tree, unsigned int); > > extern unsigned int rs6000_special_round_type_align (tree, unsigned int, > > unsigned int); > > extern unsigned int darwin_rs6000_special_round_type_align (tree, unsigned int, > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > > index 712dd1c460b..eed51e8d4a2 100644 > > --- a/gcc/config/rs6000/rs6000.c > > +++ b/gcc/config/rs6000/rs6000.c > > @@ -7856,6 +7856,41 @@ rs6000_special_adjust_field_align_p (tree type, unsigned > > int computed) > > return false; > > } > > > > +/* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ > > + > > +unsigned int > > +rs6000_special_adjust_field_align (tree type, unsigned int computed) > > +{ > > + /* If RECORD or UNION, recursively find the first field. */ > > + while (TREE_CODE (type) == RECORD_TYPE > > + || TREE_CODE (type) == UNION_TYPE > > + || TREE_CODE (type) == QUAL_UNION_TYPE) > > RECORD_OR_UNION_TYPE_P (type) > > > + { > > + tree field = TYPE_FIELDS (type); > > + > > + /* Skip all non field decls */ > > + while (field != NULL > > + && (TREE_CODE (field) != FIELD_DECL > > + || DECL_FIELD_ABI_IGNORED (field))) > > + field = DECL_CHAIN (field); > > + > > + if (field != NULL && field != type) > > when is the field (a FIELD_DECL) ever pointer equal to the containing > record type? Unless you ment to check for sth specific I suggest > to drop field != type. > > > + type = TREE_TYPE (field); > > + else > > + break; > > + } > > + > > + /* Strip arrays. */ > > + while (TREE_CODE (type) == ARRAY_TYPE) > > + type = TREE_TYPE (type); > > + > > + if (type != error_mark_node > > + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > > + computed = MIN (computed, 32); > > + > > + return computed; > > +} > > + > > /* AIX increases natural record alignment to doubleword if the first > > field is an FP double while the FP fields remain word aligned. */ > > @@ -7864,25 +7899,33 @@ rs6000_special_round_type_align (tree type,unsigned int > > computed, > > unsigned int specified) > > { > > unsigned int align = MAX (computed, specified); > > - tree field = TYPE_FIELDS (type); > > > > - /* Skip all non field decls */ > > - while (field != NULL > > - && (TREE_CODE (field) != FIELD_DECL > > - || DECL_FIELD_ABI_IGNORED (field))) > > - field = DECL_CHAIN (field); > > - > > - if (field != NULL && field != type) > > + /* If RECORD or UNION, recursively find the first field. */ > > + while (TREE_CODE (type) == RECORD_TYPE > > + || TREE_CODE (type) == UNION_TYPE > > + || TREE_CODE (type) == QUAL_UNION_TYPE) > > same as above > > > { > > - type = TREE_TYPE (field); > > - while (TREE_CODE (type) == ARRAY_TYPE) > > - type = TREE_TYPE (type); > > + tree field = TYPE_FIELDS (type); > > + > > + /* Skip all non field decls */ > > + while (field != NULL > > + && (TREE_CODE (field) != FIELD_DECL > > + || DECL_FIELD_ABI_IGNORED (field))) > > + field = DECL_CHAIN (field); > > > > - if (type != error_mark_node > > - && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > > - align = MAX (align, 64); > > + if (field != NULL && field != type) > > + type = TREE_TYPE (field); > > + else > > + break; > > } > > > > + while (TREE_CODE (type) == ARRAY_TYPE) > > + type = TREE_TYPE (type); > > + > > + if (type != error_mark_node > > + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) > > + align = MAX (align, 64); > > it looks like the changes are duplicate and could see some factoring. 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? Richard. > Richard. > > > + > > + > > return align; > > }
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? Thanks, David
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 finialized once a type is laid out which means checking COMPLETE_TYPE_P (type) (see layout_type()s early out). 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? 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 ...) 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. I guess doing some early outs should avoid most real-world slowdowns. Btw, does XLC "behave" with the problematic case? Richard. > > Thanks, David
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. Thanks, David
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? Iain
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
On Wed, Mar 24, 2021 at 3:46 PM David Edelsohn <dje.gcc@gmail.com> wrote: > > 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? With the early exit for packed structures, it is not practical to refactor the code. The following is the revised version of the patch. Thanks, David diff --git a/gcc/config/rs6000/aix.h b/gcc/config/rs6000/aix.h index 2db50c8007f..7fccb31307b 100644 --- a/gcc/config/rs6000/aix.h +++ b/gcc/config/rs6000/aix.h @@ -223,10 +223,8 @@ /* This now supports a natural alignment mode. */ /* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \ - ((TARGET_ALIGN_NATURAL == 0 \ - && (TYPE_MODE (strip_array_types (TYPE)) == DFmode \ - || TYPE_MODE (strip_array_types (TYPE)) == DCmode)) \ - ? MIN ((COMPUTED), 32) \ + (TARGET_ALIGN_NATURAL == 0 \ + ? rs6000_special_adjust_field_align (TYPE, COMPUTED) \ : (COMPUTED)) /* AIX increases natural record alignment to doubleword if the first diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h index 203660b0a78..c44fd3d0263 100644 --- a/gcc/config/rs6000/rs6000-protos.h +++ b/gcc/config/rs6000/rs6000-protos.h @@ -227,6 +227,7 @@ address_is_prefixed (rtx addr, #ifdef TREE_CODE extern unsigned int rs6000_data_alignment (tree, unsigned int, enum data_align) ; extern bool rs6000_special_adjust_field_align_p (tree, unsigned int); +extern unsigned int rs6000_special_adjust_field_align (tree, unsigned int); extern unsigned int rs6000_special_round_type_align (tree, unsigned int, unsigned int); extern unsigned int darwin_rs6000_special_round_type_align (tree, unsigned int, diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 616dae35bae..f5c71d7925b 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -7853,7 +7853,52 @@ rs6000_special_adjust_field_align_p (tree type, unsigned int computed) return false; } -/* AIX increases natural record alignment to doubleword if the first +/* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ + +unsigned int +rs6000_special_adjust_field_align (tree type, unsigned int computed) +{ + if (computed <= 32) + return computed; + + /* Strip initial arrays. */ + if (TREE_CODE (type) == ARRAY_TYPE) + while (TREE_CODE (type) == ARRAY_TYPE) + type = TREE_TYPE (type); + + /* If RECORD or UNION, recursively find the first field. */ + while (AGGREGATE_TYPE_P (type)) + { + tree field = TYPE_FIELDS (type); + + /* Skip all non field decls */ + while (field != NULL + && (TREE_CODE (field) != FIELD_DECL + || DECL_FIELD_ABI_IGNORED (field))) + field = DECL_CHAIN (field); + + if (! field) + break; + + /* A packed field does not contribute any extra alignment. */ + if (DECL_PACKED (field)) + return computed; + + type = TREE_TYPE (field); + + /* Strip arrays. */ + while (TREE_CODE (type) == ARRAY_TYPE) + type = TREE_TYPE (type); + } + + if (! AGGREGATE_TYPE_P (type) && type != error_mark_node + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) + computed = MIN (computed, 32); + + return computed; +} + +/* AIX increases natural record alignment to doubleword if the innermost first field is an FP double while the FP fields remain word aligned. */ unsigned int @@ -7861,24 +7906,38 @@ rs6000_special_round_type_align (tree type, unsigned int computed, unsigned int specified) { unsigned int align = MAX (computed, specified); - tree field = TYPE_FIELDS (type); - /* Skip all non field decls */ - while (field != NULL - && (TREE_CODE (field) != FIELD_DECL - || DECL_FIELD_ABI_IGNORED (field))) - field = DECL_CHAIN (field); + if (TYPE_PACKED (type) || align >= 64) + return align; - if (field != NULL && field != type) + /* If RECORD or UNION, recursively find the first field. */ + do { + tree field = TYPE_FIELDS (type); + + /* Skip all non field decls */ + while (field != NULL + && (TREE_CODE (field) != FIELD_DECL + || DECL_FIELD_ABI_IGNORED (field))) + field = DECL_CHAIN (field); + + if (! field) + break; + + /* A packed field does not contribute any extra alignment. */ + if (DECL_PACKED (field)) + return align; + type = TREE_TYPE (field); + + /* Strip arrays. */ while (TREE_CODE (type) == ARRAY_TYPE) type = TREE_TYPE (type); + } while (AGGREGATE_TYPE_P (type)); - if (type != error_mark_node - && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) - align = MAX (align, 64); - } + if (! AGGREGATE_TYPE_P (type) && type != error_mark_node + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) + align = MAX (align, 64); return align; }
On Wed, Mar 24, 2021 at 4:23 PM David Edelsohn <dje.gcc@gmail.com> 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? 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
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. Yes. If I understand correctly, Darwin clamps all fields with 64 bit alignment to 32 bits, but 128 bit alignment (vector) uses the natural alignment. That makes ADJUST_FIELD_ALIGN fairly simple and ROUND_TYPE_ALIGN is run once for each type. AIX clamps float fields (double, double complex) and objects derived from those types to 32 bits, but not long long int. It's easy to test if an object wants to have 64 bit alignment. It's not easy to test if an object contains a floating point primitive type in the first field at some arbitrary recursion depth. The Darwin approach is fairly elegant, but doesn't fully conform to the Power ABI. Thanks, David
--- a/gcc/config/rs6000/aix.h +++ b/gcc/config/rs6000/aix.h @@ -223,10 +223,8 @@ /* This now supports a natural alignment mode. */ /* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \ - ((TARGET_ALIGN_NATURAL == 0 \ - && (TYPE_MODE (strip_array_types (TYPE)) == DFmode \ - || TYPE_MODE (strip_array_types (TYPE)) == DCmode)) \ - ? MIN ((COMPUTED), 32) \ + (TARGET_ALIGN_NATURAL == 0 \ + ? rs6000_special_adjust_field_align (TYPE, COMPUTED) \ : (COMPUTED)) /* AIX increases natural record alignment to doubleword if the first diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h index 203660b0a78..c44fd3d0263 100644 --- a/gcc/config/rs6000/rs6000-protos.h +++ b/gcc/config/rs6000/rs6000-protos.h @@ -227,6 +227,7 @@ address_is_prefixed (rtx addr, #ifdef TREE_CODE extern unsigned int rs6000_data_alignment (tree, unsigned int, enum data_align) ; extern bool rs6000_special_adjust_field_align_p (tree, unsigned int); +extern unsigned int rs6000_special_adjust_field_align (tree, unsigned int); extern unsigned int rs6000_special_round_type_align (tree, unsigned int, unsigned int); extern unsigned int darwin_rs6000_special_round_type_align (tree, unsigned int, diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 712dd1c460b..eed51e8d4a2 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -7856,6 +7856,41 @@ rs6000_special_adjust_field_align_p (tree type, unsigned int computed) return false; } +/* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */ + +unsigned int +rs6000_special_adjust_field_align (tree type, unsigned int computed) +{ + /* If RECORD or UNION, recursively find the first field. */ + while (TREE_CODE (type) == RECORD_TYPE + || TREE_CODE (type) == UNION_TYPE + || TREE_CODE (type) == QUAL_UNION_TYPE) + { + tree field = TYPE_FIELDS (type); + + /* Skip all non field decls */ + while (field != NULL + && (TREE_CODE (field) != FIELD_DECL + || DECL_FIELD_ABI_IGNORED (field))) + field = DECL_CHAIN (field); + + if (field != NULL && field != type) + type = TREE_TYPE (field); + else + break; + } + + /* Strip arrays. */ + while (TREE_CODE (type) == ARRAY_TYPE) + type = TREE_TYPE (type); + + if (type != error_mark_node + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) + computed = MIN (computed, 32); + + return computed; +} + /* AIX increases natural record alignment to doubleword if the first field is an FP double while the FP fields remain word aligned. */ @@ -7864,25 +7899,33 @@ rs6000_special_round_type_align (tree type,unsigned int computed, unsigned int specified) { unsigned int align = MAX (computed, specified); - tree field = TYPE_FIELDS (type); - /* Skip all non field decls */ - while (field != NULL - && (TREE_CODE (field) != FIELD_DECL - || DECL_FIELD_ABI_IGNORED (field))) - field = DECL_CHAIN (field); - - if (field != NULL && field != type) + /* If RECORD or UNION, recursively find the first field. */ + while (TREE_CODE (type) == RECORD_TYPE + || TREE_CODE (type) == UNION_TYPE + || TREE_CODE (type) == QUAL_UNION_TYPE) { - type = TREE_TYPE (field); - while (TREE_CODE (type) == ARRAY_TYPE) - type = TREE_TYPE (type); + tree field = TYPE_FIELDS (type); + + /* Skip all non field decls */ + while (field != NULL + && (TREE_CODE (field) != FIELD_DECL + || DECL_FIELD_ABI_IGNORED (field))) + field = DECL_CHAIN (field); - if (type != error_mark_node - && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) - align = MAX (align, 64); + if (field != NULL && field != type) + type = TREE_TYPE (field); + else + break; } + while (TREE_CODE (type) == ARRAY_TYPE) + type = TREE_TYPE (type); + + if (type != error_mark_node + && (TYPE_MODE (type) == DFmode || TYPE_MODE (type) == DCmode)) + align = MAX (align, 64); + + return align; }
The AIX power alignment rules apply the natural alignment of the "first member" if it is of a floating-point data type (or is an aggregate whose recursively "first" member or element is such a type). The alignment associated with these types for subsequent members use an alignment value where the floating-point data type is considered to have 4-byte alignment. GCC had been stripping array type but had not recursively looked within structs and unions. This also applies to classes and subclasses and, therefore, becomes more prominent with C++. For example, struct A { double x[2]; int y; }; struct B { int i; struct A a; }; struct A has double-word alignment when referenced independently, but word alignment and offset within struct B despite the alignment of struct A. If struct A were the first member of struct B, struct B would have double-word alignment. One must search for the innermost first member to increase the alignment if double and then search for the innermost first member to reduce the alignment if the TYPE had double-word alignment solely because the innermost first member was double. This patch recursively looks through the first member to apply the double-word alignment to the struct / union as a whole and to apply the word alignment to the struct or union as a member within a struct or union. This is an ABI change for GCCon AIX, but GCC on AIX had not correctly implemented the AIX ABI and had not been compatible with the IBM XL compiler. If anyone can double-check that the patch walks the fields correctly and handles the error conditions correctly, it would be appreciated. Bootstrapped on powerpc-ibm-aix7.2.3.0. Thanks, David * config/rs6000/aix.h (ADJUST_FIELD_ALIGN): Call function. * config/rs6000/rs6000-protos.h: Declare. * config/rs6000/rs6000.c: Define. index 2db50c8007f..7fccb31307b 100644