| Message ID | 20101028173314.GA13514@intel.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 28, 2010 at 10:33:14AM -0700, H.J. Lu wrote: > Hi, > > On Darwin, although long double is aligned to 16byte, it is aligned to > 4byte when passed on stack. > > Jack, please try this on Darwin. OK for trunk if there are no > regressions on Darwin and Linux? H.J., I can confirm that all of the regressions introduced by r165965 are eliminated on x86_64-apple-darwin10... http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg02294.html Thanks for fixing this issue. Jack > > Thanks. > > > H.J. > --- > 2010-10-28 H.J. Lu <hongjiu.lu@intel.com> > > PR target/46195 > * config/i386/i386.c (bigger_than_4byte_aligned_p): New. > (ix86_function_arg_boundary): Align long double parameters on > stack to 4byte in 32bit. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f2bd705..02650df 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -7032,6 +7032,59 @@ ix86_old_function_arg_boundary (enum machine_mode mode, const_tree type, > return align; > } > > +/* Return true when aggregate TYPE should be aligned at bigger than > + 4byte for 32bit argument passing ABI. */ > + > +static bool > +bigger_than_4byte_aligned_p (const_tree type) > +{ > + enum machine_mode mode = TYPE_MODE (type); > + > + if (mode == XFmode || mode == XCmode) > + return false; > + > + if (TYPE_ALIGN (type) < 128) > + return false; > + > + if (!AGGREGATE_TYPE_P (type)) > + return TYPE_ALIGN (type) >= 128; > + else > + { > + /* Walk the aggregates recursively. */ > + switch (TREE_CODE (type)) > + { > + case RECORD_TYPE: > + case UNION_TYPE: > + case QUAL_UNION_TYPE: > + { > + tree field; > + > + /* Walk all the structure fields. */ > + for (field = TYPE_FIELDS (type); > + field; > + field = DECL_CHAIN (field)) > + { > + if (TREE_CODE (field) == FIELD_DECL > + && bigger_than_4byte_aligned_p (TREE_TYPE (field))) > + return true; > + } > + break; > + } > + > + case ARRAY_TYPE: > + /* Just for use if some languages passes arrays by value. */ > + if (bigger_than_4byte_aligned_p (TREE_TYPE (type))) > + return true; > + break; > + > + default: > + gcc_unreachable (); > + } > + } > + > + return false; > +} > + > /* Gives the alignment boundary, in bits, of an argument with the > specified mode and type. */ > > @@ -7055,7 +7108,18 @@ ix86_function_arg_boundary (enum machine_mode mode, const_tree type) > static bool warned; > int saved_align = align; > > - if (!TARGET_64BIT && align < 128) > + /* i386 ABI defines all arguments to be 4 byte aligned. Even if > + long double is aligned to 16 byte, we always align it at 4 > + byte, whether it is a scalar or the part of aggregate, when > + passed as function argument. */ > + if (!TARGET_64BIT > + && (align < 128 > + || (align == 128 > + && (mode == XFmode > + || mode == XCmode > + || (type > + && AGGREGATE_TYPE_P (type) > + && !bigger_than_4byte_aligned_p (type)))))) > align = PARM_BOUNDARY; > > if (warn_psabi
On Thu, Oct 28, 2010 at 7:11 PM, Jack Howarth <howarth@bromo.med.uc.edu> wrote: > On Thu, Oct 28, 2010 at 10:33:14AM -0700, H.J. Lu wrote: >> Hi, >> >> On Darwin, although long double is aligned to 16byte, it is aligned to >> 4byte when passed on stack. >> >> Jack, please try this on Darwin. OK for trunk if there are no >> regressions on Darwin and Linux? > > H.J., > I can confirm that all of the regressions introduced by r165965 > are eliminated on x86_64-apple-darwin10... > > http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg02294.html > > Thanks for fixing this issue. > Jack > >> >> Thanks. >> >> >> H.J. >> --- >> 2010-10-28 H.J. Lu <hongjiu.lu@intel.com> >> >> PR target/46195 >> * config/i386/i386.c (bigger_than_4byte_aligned_p): New. >> (ix86_function_arg_boundary): Align long double parameters on >> stack to 4byte in 32bit. There are regressions on Linux nor Darwin. OK for trunk? Thanks.
On Oct 28, 2010, at 10:42 PM, H.J. Lu wrote:
> OK for trunk?
I'm fine with the darwin aspects of this, though, I'd like an x86/abi
type person to review it. I mention this so that they don't wish for
a darwin person to review it and remain silent.. :-) Thanks.
On Thu, Oct 28, 2010 at 8:02 PM, Mike Stump <mikestump@comcast.net> wrote: > On Oct 28, 2010, at 10:42 PM, H.J. Lu wrote: >> >> OK for trunk? > > I'm fine with the darwin aspects of this, though, I'd like an x86/abi type > person to review it. I mention this so that they don't wish for a darwin > person to review it and remain silent.. :-) Thanks. > That is an issue only on 32bit Darwin since 32bit Darwin aligns long double to 16byte and passes it on stack with 4byte alignment. Other targets always use 4byte alignment.
On Thu, Oct 28, 2010 at 7:33 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > On Darwin, although long double is aligned to 16byte, it is aligned to > 4byte when passed on stack. > > Jack, please try this on Darwin. OK for trunk if there are no > regressions on Darwin and Linux? > > Thanks. > > > H.J. > --- > 2010-10-28 H.J. Lu <hongjiu.lu@intel.com> > > PR target/46195 > * config/i386/i386.c (bigger_than_4byte_aligned_p): New. > (ix86_function_arg_boundary): Align long double parameters on > stack to 4byte in 32bit. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f2bd705..02650df 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -7032,6 +7032,59 @@ ix86_old_function_arg_boundary (enum machine_mode mode, const_tree type, > return align; > } > > +/* Return true when aggregate TYPE should be aligned at bigger than > + 4byte for 32bit argument passing ABI. */ > + > +static bool > +bigger_than_4byte_aligned_p (const_tree type) I propose to change the function prototype and name to: int ix86_alignment_needed (const_tree_type) and this function will return alignment (in bits), as is customary in gcc. > +{ > + enum machine_mode mode = TYPE_MODE (type); > + > + if (mode == XFmode || mode == XCmode) > + return false; > + > + if (TYPE_ALIGN (type) < 128) > + return false; > + > + if (!AGGREGATE_TYPE_P (type)) > + return TYPE_ALIGN (type) >= 128; > + else > + { > + /* Walk the aggregates recursively. */ > + switch (TREE_CODE (type)) > + { > + case RECORD_TYPE: > + case UNION_TYPE: > + case QUAL_UNION_TYPE: > + { > + tree field; > + > + /* Walk all the structure fields. */ > + for (field = TYPE_FIELDS (type); > + field; > + field = DECL_CHAIN (field)) > + { > + if (TREE_CODE (field) == FIELD_DECL > + && bigger_than_4byte_aligned_p (TREE_TYPE (field))) > + return true; > + } > + break; > + } > + > + case ARRAY_TYPE: > + /* Just for use if some languages passes arrays by value. */ > + if (bigger_than_4byte_aligned_p (TREE_TYPE (type))) > + return true; > + break; > + > + default: > + gcc_unreachable (); > + } > + } > + > + return false; > +} > + > /* Gives the alignment boundary, in bits, of an argument with the > specified mode and type. */ > > @@ -7055,7 +7108,18 @@ ix86_function_arg_boundary (enum machine_mode mode, const_tree type) > static bool warned; > int saved_align = align; > > - if (!TARGET_64BIT && align < 128) > + /* i386 ABI defines all arguments to be 4 byte aligned. Even if > + long double is aligned to 16 byte, we always align it at 4 > + byte, whether it is a scalar or the part of aggregate, when > + passed as function argument. */ > + if (!TARGET_64BIT > + && (align < 128 > + || (align == 128 > + && (mode == XFmode > + || mode == XCmode > + || (type > + && AGGREGATE_TYPE_P (type) > + && !bigger_than_4byte_aligned_p (type)))))) > align = PARM_BOUNDARY; There is something wrong with the usage of utility function if it has to be surrounded by soo much extra checks. I propose to change all this to: align = ix86_alignment_needed (type) where all the knowledge of type alignment will be moved into the ix86_alignment_needed function. Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index f2bd705..02650df 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -7032,6 +7032,59 @@ ix86_old_function_arg_boundary (enum machine_mode mode, const_tree type, return align; } +/* Return true when aggregate TYPE should be aligned at bigger than + 4byte for 32bit argument passing ABI. */ + +static bool +bigger_than_4byte_aligned_p (const_tree type) +{ + enum machine_mode mode = TYPE_MODE (type); + + if (mode == XFmode || mode == XCmode) + return false; + + if (TYPE_ALIGN (type) < 128) + return false; + + if (!AGGREGATE_TYPE_P (type)) + return TYPE_ALIGN (type) >= 128; + else + { + /* Walk the aggregates recursively. */ + switch (TREE_CODE (type)) + { + case RECORD_TYPE: + case UNION_TYPE: + case QUAL_UNION_TYPE: + { + tree field; + + /* Walk all the structure fields. */ + for (field = TYPE_FIELDS (type); + field; + field = DECL_CHAIN (field)) + { + if (TREE_CODE (field) == FIELD_DECL + && bigger_than_4byte_aligned_p (TREE_TYPE (field))) + return true; + } + break; + } + + case ARRAY_TYPE: + /* Just for use if some languages passes arrays by value. */ + if (bigger_than_4byte_aligned_p (TREE_TYPE (type))) + return true; + break; + + default: + gcc_unreachable (); + } + } + + return false; +} + /* Gives the alignment boundary, in bits, of an argument with the specified mode and type. */ @@ -7055,7 +7108,18 @@ ix86_function_arg_boundary (enum machine_mode mode, const_tree type) static bool warned; int saved_align = align; - if (!TARGET_64BIT && align < 128) + /* i386 ABI defines all arguments to be 4 byte aligned. Even if + long double is aligned to 16 byte, we always align it at 4 + byte, whether it is a scalar or the part of aggregate, when + passed as function argument. */ + if (!TARGET_64BIT + && (align < 128 + || (align == 128 + && (mode == XFmode + || mode == XCmode + || (type + && AGGREGATE_TYPE_P (type) + && !bigger_than_4byte_aligned_p (type)))))) align = PARM_BOUNDARY; if (warn_psabi