This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR79278, amend ADJUST_FIELD_ALIGN interface
- From: Richard Biener <rguenther at suse dot de>
- To: Iain Sandoe <iain at codesourcery dot com>
- Cc: Segher Boessenkool <segher at kernel dot crashing dot org>, Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>, ian at airs dot com, nickc at redhat dot com, aoliva at redhat dot com, gnu at amylaar dot uk, ubizjak at gmail dot com, dje dot gcc at gmail dot com
- Date: Tue, 7 Feb 2017 13:25:18 +0100 (CET)
- Subject: Re: [PATCH] Fix PR79278, amend ADJUST_FIELD_ALIGN interface
- Authentication-results: sourceware.org; auth=none
- References: <alpine.LSU.2.20.1701310954080.12993@r111.fhfr.qr> <52761145-a884-f356-8e28-0d3ad00b32cd@redhat.com> <alpine.LSU.2.20.1702010927120.12993@r111.fhfr.qr> <27D06627-B29A-441B-87DD-971C4CBEF2B9@codesourcery.com> <alpine.LSU.2.20.1702011156140.12993@r111.fhfr.qr> <20170201215306.GL21840@gate.crashing.org> <20170202024555.GN21840@gate.crashing.org> <alpine.LSU.2.20.1702020905000.12993@r111.fhfr.qr> <017F6558-F7BC-473C-92EA-95BA1CDE3334@codesourcery.com>
On Fri, 3 Feb 2017, Iain Sandoe wrote:
>
> > On 2 Feb 2017, at 08:08, Richard Biener <rguenther@suse.de> wrote:
> >
> > On Wed, 1 Feb 2017, Segher Boessenkool wrote:
> >
> >> On Wed, Feb 01, 2017 at 03:53:09PM -0600, Segher Boessenkool wrote:
> >>> On Wed, Feb 01, 2017 at 11:59:10AM +0100, Richard Biener wrote:
> >>>> Wasn't successful in making a cross to ppc64-linux build its libobjc.
> >>>
> >>> I'll do a native build. Just the patch in the first message in this
> >>> thread? And just running the testsuite is enough, or is there
> >>> something specific you want tested?
> >>
> >> Done now. No new failures on powerpc64-linux {-m32,-m64}. I needed
> >> the following additional patch; I do not know if that works on other
> >> targets (or if it actually is correct!)
> >
> > Yes, I knew I needed something there but didn't know what is correct
> > either ;) But I concluded as 'type' is const char * here which is
> > totally broken it probably doesn't matter what we pass as 2nd argument
> > either ...
> >
> > So, consider the patch changed like Segher proposed below (and thanks
> > Segher for the testing). For reference attached again below, including
> > tm.texi.in.
> >
> > Barring further comments I'll check this in at the beginning of next
> > week.
>
> I bootstrapped (+ada +obj-c++) 245116+this patch on x86_64-darwin15 and
> powerpc-darwin9, I don’t see any regressions on the objective-c or
> objective-c++ suites (which include testing -fgnu-runtime on Darwin).
Now applied as r245245 with the earlier PR79256 fix (partly) reverted.
Richard.
> Iain
>
> >
> > Thanks,
> > Richard.
> >
> > 2017-02-02 Richard Biener <rguenther@suse.de>
> >
> > PR tree-optimization/79256
> > PR middle-end/79278
> > * builtins.c (get_object_alignment_2): Use min_align_of_type
> > to extract alignment for MEM_REFs to honor BIGGEST_FIELD_ALIGNMENT
> > and ADJUST_FIELD_ALIGN.
> >
> > * doc/tm.texi.in (ADJUST_FIELD_ALIGN): Adjust to take additional
> > type parameter.
> > * doc/tm.texi: Regenerate.
> > * stor-layout.c (layout_decl): Adjust.
> > (update_alignment_for_field): Likewise.
> > (place_field): Likewise.
> > (min_align_of_type): Likewise.
> > * config/arc/arc.h (ADJUST_FIELD_ALIGN): Adjust.
> > * config/epiphany/epiphany.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/epiphany/epiphany.c (epiphany_adjust_field_align): Likewise.
> > * config/frv/frv.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/frv/frv.c (frv_adjust_field_align): Likewise.
> > * config/i386/i386.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/i386/i386.c (x86_field_alignment): Likewise.
> > * config/rs6000/aix.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/rs6000/darwin.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/rs6000/freebsd64.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/rs6000/linux64.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/rs6000/sysv4.h (ADJUST_FIELD_ALIGN): Likewise.
> > * config/rs6000/rs6000.c (rs6000_special_adjust_field_align_p):
> > Likewise.
> >
> > go/
> > * go-backend.c (go_field_alignment): Adjust.
> >
> > libobjc/
> > * encoding.c (objc_layout_structure_next_member): Adjust
> > ADJUST_FIELD_ALIGN usage.
> >
> > Index: gcc/builtins.c
> > ===================================================================
> > --- gcc/builtins.c (revision 245115)
> > +++ gcc/builtins.c (working copy)
> > @@ -334,9 +334,11 @@ get_object_alignment_2 (tree exp, unsign
> > Do so only if get_pointer_alignment_1 did not reveal absolute
> > alignment knowledge and if using that alignment would
> > improve the situation. */
> > + unsigned int talign;
> > if (!addr_p && !known_alignment
> > - && TYPE_ALIGN (TREE_TYPE (exp)) > align)
> > - align = TYPE_ALIGN (TREE_TYPE (exp));
> > + && (talign = min_align_of_type (TREE_TYPE (exp)) * BITS_PER_UNIT)
> > + && talign > align)
> > + align = talign;
> > else
> > {
> > /* Else adjust bitpos accordingly. */
> > Index: gcc/config/arc/arc.h
> > ===================================================================
> > --- gcc/config/arc/arc.h (revision 245115)
> > +++ gcc/config/arc/arc.h (working copy)
> > @@ -317,8 +317,8 @@ if (GET_MODE_CLASS (MODE) == MODE_INT \
> > construct.
> > */
> >
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > -(TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > +(TYPE_MODE (strip_array_types (TYPE)) == DFmode \
> > ? MIN ((COMPUTED), 32) : (COMPUTED))
> >
> >
> > Index: gcc/config/epiphany/epiphany.c
> > ===================================================================
> > --- gcc/config/epiphany/epiphany.c (revision 245115)
> > +++ gcc/config/epiphany/epiphany.c (working copy)
> > @@ -2855,12 +2855,12 @@ epiphany_special_round_type_align (tree
> > arrays-at-the-end-of-structs work, like for struct gcov_fn_info in
> > libgcov.c . */
> > unsigned
> > -epiphany_adjust_field_align (tree field, unsigned computed)
> > +epiphany_adjust_field_align (tree type, unsigned computed)
> > {
> > if (computed == 32
> > - && TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE)
> > + && TREE_CODE (type) == ARRAY_TYPE)
> > {
> > - tree elmsz = TYPE_SIZE (TREE_TYPE (TREE_TYPE (field)));
> > + tree elmsz = TYPE_SIZE (TREE_TYPE (type));
> >
> > if (!tree_fits_uhwi_p (elmsz) || tree_to_uhwi (elmsz) >= 32)
> > return 64;
> > Index: gcc/config/epiphany/epiphany.h
> > ===================================================================
> > --- gcc/config/epiphany/epiphany.h (revision 245115)
> > +++ gcc/config/epiphany/epiphany.h (working copy)
> > @@ -187,8 +187,8 @@ along with GCC; see the file COPYING3.
> > (SPECIFIED_ALIGN)) \
> > : MAX ((MANGLED_ALIGN), (SPECIFIED_ALIGN)))
> >
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > - epiphany_adjust_field_align((FIELD), (COMPUTED))
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > + epiphany_adjust_field_align((TYPE), (COMPUTED))
> >
> > /* Layout of source language data types. */
> >
> > Index: gcc/config/frv/frv.c
> > ===================================================================
> > --- gcc/config/frv/frv.c (revision 245115)
> > +++ gcc/config/frv/frv.c (working copy)
> > @@ -6482,7 +6482,8 @@ int
> > frv_adjust_field_align (tree field, int computed)
> > {
> > /* Make sure that the bitfield is not wider than the type. */
> > - if (DECL_BIT_FIELD (field)
> > + if (field
> > + && DECL_BIT_FIELD (field)
> > && !DECL_ARTIFICIAL (field))
> > {
> > tree parent = DECL_CONTEXT (field);
> > Index: gcc/config/frv/frv.h
> > ===================================================================
> > --- gcc/config/frv/frv.h (revision 245115)
> > +++ gcc/config/frv/frv.h (working copy)
> > @@ -331,7 +331,7 @@
> > alignment computed in the usual way is COMPUTED. GCC uses this
> > value instead of the value in `BIGGEST_ALIGNMENT' or
> > `BIGGEST_FIELD_ALIGNMENT', if defined, for structure fields only. */
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > frv_adjust_field_align (FIELD, COMPUTED)
> > #endif
> >
> > Index: gcc/config/i386/i386.c
> > ===================================================================
> > --- gcc/config/i386/i386.c (revision 245115)
> > +++ gcc/config/i386/i386.c (working copy)
> > @@ -41676,10 +41676,9 @@ x86_file_start (void)
> > }
> >
> > int
> > -x86_field_alignment (tree field, int computed)
> > +x86_field_alignment (tree type, int computed)
> > {
> > machine_mode mode;
> > - tree type = TREE_TYPE (field);
> >
> > if (TARGET_64BIT || TARGET_ALIGN_DOUBLE)
> > return computed;
> > Index: gcc/config/i386/i386.h
> > ===================================================================
> > --- gcc/config/i386/i386.h (revision 245115)
> > +++ gcc/config/i386/i386.h (working copy)
> > @@ -847,8 +847,8 @@ extern const char *host_detect_local_cpu
> > #define BIGGEST_FIELD_ALIGNMENT 32
> > #endif
> > #else
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > - x86_field_alignment ((FIELD), (COMPUTED))
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > + x86_field_alignment ((TYPE), (COMPUTED))
> > #endif
> >
> > /* If defined, a C expression to compute the alignment given to a
> > Index: gcc/config/rs6000/aix.h
> > ===================================================================
> > --- gcc/config/rs6000/aix.h (revision 245115)
> > +++ gcc/config/rs6000/aix.h (working copy)
> > @@ -218,9 +218,9 @@
> >
> > /* This now supports a natural alignment mode. */
> > /* AIX word-aligns FP doubles but doubleword-aligns 64-bit ints. */
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > ((TARGET_ALIGN_NATURAL == 0 \
> > - && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode) \
> > + && TYPE_MODE (strip_array_types (TYPE)) == DFmode) \
> > ? MIN ((COMPUTED), 32) \
> > : (COMPUTED))
> >
> > Index: gcc/config/rs6000/darwin.h
> > ===================================================================
> > --- gcc/config/rs6000/darwin.h (revision 245115)
> > +++ gcc/config/rs6000/darwin.h (working copy)
> > @@ -319,7 +319,7 @@ extern int darwin_emit_branch_islands;
> > suppressed for vector and long double items (both 128 in size).
> > There is a dummy use of the FIELD argument to avoid an unused variable
> > warning (see PR59496). */
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > ((void) (FIELD), \
> > (TARGET_ALIGN_NATURAL \
> > ? (COMPUTED) \
> > Index: gcc/config/rs6000/freebsd64.h
> > ===================================================================
> > --- gcc/config/rs6000/freebsd64.h (revision 245115)
> > +++ gcc/config/rs6000/freebsd64.h (working copy)
> > @@ -365,12 +365,12 @@ extern int dot_symbols;
> >
> > /* PowerPC64 Linux word-aligns FP doubles when -malign-power is given. */
> > #undef ADJUST_FIELD_ALIGN
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > - (rs6000_special_adjust_field_align_p ((FIELD), (COMPUTED)) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > + (rs6000_special_adjust_field_align_p ((TYPE), (COMPUTED)) \
> > ? 128 \
> > : (TARGET_64BIT \
> > && TARGET_ALIGN_NATURAL == 0 \
> > - && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode) \
> > + && TYPE_MODE (strip_array_types (TYPE)) == DFmode) \
> > ? MIN ((COMPUTED), 32) \
> > : (COMPUTED))
> >
> > Index: gcc/config/rs6000/linux64.h
> > ===================================================================
> > --- gcc/config/rs6000/linux64.h (revision 245115)
> > +++ gcc/config/rs6000/linux64.h (working copy)
> > @@ -292,12 +292,12 @@ extern int dot_symbols;
> >
> > /* PowerPC64 Linux word-aligns FP doubles when -malign-power is given. */
> > #undef ADJUST_FIELD_ALIGN
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > - (rs6000_special_adjust_field_align_p ((FIELD), (COMPUTED)) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > + (rs6000_special_adjust_field_align_p ((TYPE), (COMPUTED)) \
> > ? 128 \
> > : (TARGET_64BIT \
> > && TARGET_ALIGN_NATURAL == 0 \
> > - && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode) \
> > + && TYPE_MODE (strip_array_types (TYPE)) == DFmode) \
> > ? MIN ((COMPUTED), 32) \
> > : (COMPUTED))
> >
> > Index: gcc/config/rs6000/rs6000.c
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.c (revision 245115)
> > +++ gcc/config/rs6000/rs6000.c (working copy)
> > @@ -7972,9 +7972,9 @@ rs6000_data_alignment (tree type, unsign
> > /* Previous GCC releases forced all vector types to have 16-byte alignment. */
> >
> > bool
> > -rs6000_special_adjust_field_align_p (tree field, unsigned int computed)
> > +rs6000_special_adjust_field_align_p (tree type, unsigned int computed)
> > {
> > - if (TARGET_ALTIVEC && TREE_CODE (TREE_TYPE (field)) == VECTOR_TYPE)
> > + if (TARGET_ALTIVEC && TREE_CODE (type) == VECTOR_TYPE)
> > {
> > if (computed != 128)
> > {
> > Index: gcc/config/rs6000/sysv4.h
> > ===================================================================
> > --- gcc/config/rs6000/sysv4.h (revision 245115)
> > +++ gcc/config/rs6000/sysv4.h (working copy)
> > @@ -298,8 +298,8 @@ do { \
> >
> > /* An expression for the alignment of a structure field FIELD if the
> > alignment computed in the usual way is COMPUTED. */
> > -#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
> > - (rs6000_special_adjust_field_align_p ((FIELD), (COMPUTED)) \
> > +#define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
> > + (rs6000_special_adjust_field_align_p ((TYPE), (COMPUTED)) \
> > ? 128 : COMPUTED)
> >
> > #undef BIGGEST_FIELD_ALIGNMENT
> > Index: gcc/doc/tm.texi
> > ===================================================================
> > --- gcc/doc/tm.texi (revision 245115)
> > +++ gcc/doc/tm.texi (working copy)
> > @@ -1042,13 +1042,15 @@ structure and union fields only, unless
> > by the @code{__attribute__ ((aligned (@var{n})))} construct.
> > @end defmac
> >
> > -@defmac ADJUST_FIELD_ALIGN (@var{field}, @var{computed})
> > -An expression for the alignment of a structure field @var{field} if the
> > -alignment computed in the usual way (including applying of
> > -@code{BIGGEST_ALIGNMENT} and @code{BIGGEST_FIELD_ALIGNMENT} to the
> > +@defmac ADJUST_FIELD_ALIGN (@var{field}, @var{type}, @var{computed})
> > +An expression for the alignment of a structure field @var{field} of
> > +type @var{type} if the alignment computed in the usual way (including
> > +applying of @code{BIGGEST_ALIGNMENT} and @code{BIGGEST_FIELD_ALIGNMENT} to the
> > alignment) is @var{computed}. It overrides alignment only if the
> > field alignment has not been set by the
> > -@code{__attribute__ ((aligned (@var{n})))} construct.
> > +@code{__attribute__ ((aligned (@var{n})))} construct. Note that @var{field}
> > +may be @code{NULL_TREE} in case we just query for the minimum alignment
> > +of a field of type @var{type} in structure context.
> > @end defmac
> >
> > @defmac MAX_STACK_ALIGNMENT
> > Index: gcc/doc/tm.texi.in
> > ===================================================================
> > --- gcc/doc/tm.texi.in (revision 245115)
> > +++ gcc/doc/tm.texi.in (working copy)
> > @@ -990,13 +990,15 @@ structure and union fields only, unless
> > by the @code{__attribute__ ((aligned (@var{n})))} construct.
> > @end defmac
> >
> > -@defmac ADJUST_FIELD_ALIGN (@var{field}, @var{computed})
> > -An expression for the alignment of a structure field @var{field} if the
> > -alignment computed in the usual way (including applying of
> > -@code{BIGGEST_ALIGNMENT} and @code{BIGGEST_FIELD_ALIGNMENT} to the
> > +@defmac ADJUST_FIELD_ALIGN (@var{field}, @var{type}, @var{computed})
> > +An expression for the alignment of a structure field @var{field} of
> > +type @var{type} if the alignment computed in the usual way (including
> > +applying of @code{BIGGEST_ALIGNMENT} and @code{BIGGEST_FIELD_ALIGNMENT} to the
> > alignment) is @var{computed}. It overrides alignment only if the
> > field alignment has not been set by the
> > -@code{__attribute__ ((aligned (@var{n})))} construct.
> > +@code{__attribute__ ((aligned (@var{n})))} construct. Note that @var{field}
> > +may be @code{NULL_TREE} in case we just query for the minimum alignment
> > +of a field of type @var{type} in structure context.
> > @end defmac
> >
> > @defmac MAX_STACK_ALIGNMENT
> > Index: gcc/go/go-backend.c
> > ===================================================================
> > --- gcc/go/go-backend.c (revision 245115)
> > +++ gcc/go/go-backend.c (working copy)
> > @@ -71,11 +71,7 @@ go_field_alignment (tree t)
> > #endif
> >
> > #ifdef ADJUST_FIELD_ALIGN
> > - {
> > - tree field ATTRIBUTE_UNUSED;
> > - field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL, t);
> > - v = ADJUST_FIELD_ALIGN (field, v);
> > - }
> > + v = ADJUST_FIELD_ALIGN (NULL_TREE, t, v);
> > #endif
> >
> > return v / BITS_PER_UNIT;
> > Index: gcc/stor-layout.c
> > ===================================================================
> > --- gcc/stor-layout.c (revision 245115)
> > +++ gcc/stor-layout.c (working copy)
> > @@ -718,7 +718,8 @@ layout_decl (tree decl, unsigned int kno
> > (unsigned) BIGGEST_FIELD_ALIGNMENT));
> > #endif
> > #ifdef ADJUST_FIELD_ALIGN
> > - SET_DECL_ALIGN (decl, ADJUST_FIELD_ALIGN (decl, DECL_ALIGN (decl)));
> > + SET_DECL_ALIGN (decl, ADJUST_FIELD_ALIGN (decl, TREE_TYPE (decl),
> > + DECL_ALIGN (decl)));
> > #endif
> > }
> >
> > @@ -1032,7 +1033,7 @@ update_alignment_for_field (record_layou
> >
> > #ifdef ADJUST_FIELD_ALIGN
> > if (! TYPE_USER_ALIGN (type))
> > - type_align = ADJUST_FIELD_ALIGN (field, type_align);
> > + type_align = ADJUST_FIELD_ALIGN (field, type, type_align);
> > #endif
> >
> > /* Targets might chose to handle unnamed and hence possibly
> > @@ -1260,7 +1261,7 @@ place_field (record_layout_info rli, tre
> >
> > #ifdef ADJUST_FIELD_ALIGN
> > if (! TYPE_USER_ALIGN (type))
> > - type_align = ADJUST_FIELD_ALIGN (field, type_align);
> > + type_align = ADJUST_FIELD_ALIGN (field, type, type_align);
> > #endif
> >
> > /* A bit field may not span more units of alignment of its type
> > @@ -1303,7 +1304,7 @@ place_field (record_layout_info rli, tre
> >
> > #ifdef ADJUST_FIELD_ALIGN
> > if (! TYPE_USER_ALIGN (type))
> > - type_align = ADJUST_FIELD_ALIGN (field, type_align);
> > + type_align = ADJUST_FIELD_ALIGN (field, type, type_align);
> > #endif
> >
> > if (maximum_field_alignment != 0)
> > @@ -2411,9 +2412,7 @@ min_align_of_type (tree type)
> > #endif
> > unsigned int field_align = align;
> > #ifdef ADJUST_FIELD_ALIGN
> > - tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE, type);
> > - field_align = ADJUST_FIELD_ALIGN (field, field_align);
> > - ggc_free (field);
> > + field_align = ADJUST_FIELD_ALIGN (NULL_TREE, type, field_align);
> > #endif
> > align = MIN (align, field_align);
> > }
> > Index: libobjc/encoding.c
> > ===================================================================
> > --- libobjc/encoding.c (revision 245115)
> > +++ libobjc/encoding.c (working copy)
> > @@ -1159,7 +1159,7 @@ objc_layout_structure_next_member (struc
> > desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT);
> > #endif
> > #ifdef ADJUST_FIELD_ALIGN
> > - desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
> > + desired_align = ADJUST_FIELD_ALIGN (type, type, desired_align);
> > #endif
> >
> > /* Record must have at least as much alignment as any field.
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)