| Message ID | 1399073674.30798.4.camel@otta |
|---|---|
| State | New |
| Headers | show |
On Fri, 2014-05-02 at 18:34 -0500, Peter Bergner wrote: > This passed bootstrap and regtesting on powerpc64-linux with no regressions > and the pack0[23].c tests now pass. Ok for trunk? > > Is this also ok for the 4.8/4.9 branches once my testing on those are complete? FYI, this passed bootstrap and regtesting on both the 4.8 and 4.9 builds with no regressions. Peter
On Fri, May 2, 2014 at 7:34 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > Currently, the gcc.target/powerpc/pack0[23].c test cases are failing on trunk > and the 4.9 and 4.8 branches. The problem is we either fail to register the > builtins leading to undefined reference errors to the __builtin_* symbol or > we ICE due to having registered the builtin, but due to compiler options, > rs6000_expand_builtin doesn't think we should expanding the builtin. I fixed > the new pack/unpack long double builtins by enabling them for TARGET_HARD_FLOAT > and easing the gcc_assert in rs6000_expand_builtin(). > > The pack/unpack DFP builtins just required passing -mhard-dfp as an option > to the test case. I also reduced the dg-require-effective-target, since > the test case works just fine on non-vsx POWER6 hardware. In the process, > I noticed we didn't disallow -msoft-float -mhard-dfp being used together, > so we enforce that now. > > I'll note this is a precursor patch to using the __builtin_pack_longdouble > builtin in libgcc/config/rs6000/ibm-ldouble.c which currently uses code > that copies data through a union. The code it produces is horrific, as > it stores the two double fprs to memory, loads them into gprs, then stores > them back to memory before loading them back into fprs again. The builtin > will also us to get a simple fmr(s). > > This passed bootstrap and regtesting on powerpc64-linux with no regressions > and the pack0[23].c tests now pass. Ok for trunk? > > Is this also ok for the 4.8/4.9 branches once my testing on those are complete? > > Peter > > > gcc/ > * config/rs6000/rs6000.h (RS6000_BTM_HARD_FLOAT): New define. > (RS6000_BTM_COMMON): Add RS6000_BTM_HARD_FLOAT. > (TARGET_EXTRA_BUILTINS): Add TARGET_HARD_FLOAT. > * config/rs6000/rs6000-builtin.def (BU_MISC_1): > Use RS6000_BTM_HARD_FLOAT. > (BU_MISC_2): Likewise. > * config/rs6000/rs6000.c (rs6000_builtin_mask_calculate): Handle > RS6000_BTM_HARD_FLOAT. > (rs6000_option_override_internal): Enforce -mhard-float if -mhard-dfp > is explicitly used. > (rs6000_invalid_builtin): Add hard floating builtin support. > (rs6000_expand_builtin): Relax the gcc_assert to allow the new > hard float builtins. > (rs6000_builtin_mask_names): Add RS6000_BTM_HARD_FLOAT. > > gcc/testsuite/ > * gcc.target/powerpc/pack02.c (dg-options): Add -mhard-float. > (dg-require-effective-target): Change target to powerpc_fprs. > * gcc.target/powerpc/pack03.c (dg-options): Add -mhard-dfp. > (dg-require-effective-target): Change target to dfprt. > > Index: gcc/config/rs6000/rs6000.h > =================================================================== > --- gcc/config/rs6000/rs6000.h (revision 209990) > +++ gcc/config/rs6000/rs6000.h (working copy) > @@ -624,7 +624,8 @@ extern int rs6000_vector_align[]; > || TARGET_CMPB /* ISA 2.05 */ \ > || TARGET_POPCNTD /* ISA 2.06 */ \ > || TARGET_ALTIVEC \ > - || TARGET_VSX))) > + || TARGET_VSX \ > + || TARGET_HARD_FLOAT))) > > /* E500 cores only support plain "sync", not lwsync. */ > #define TARGET_NO_LWSYNC (rs6000_cpu == PROCESSOR_PPC8540 \ > @@ -2517,6 +2518,7 @@ extern int frame_pointer_needed; > #define RS6000_BTM_POPCNTD MASK_POPCNTD /* Target supports ISA 2.06. */ > #define RS6000_BTM_CELL MASK_FPRND /* Target is cell powerpc. */ > #define RS6000_BTM_DFP MASK_DFP /* Decimal floating point. */ > +#define RS6000_BTM_HARD_FLOAT MASK_SOFT_FLOAT /* Hardware floating point. */ It's a little confusing that the use reverses the polarity of the mask meaning. > Index: gcc/config/rs6000/rs6000.c > =================================================================== > --- gcc/config/rs6000/rs6000.c (revision 209990) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -3039,7 +3039,8 @@ rs6000_builtin_mask_calculate (void) > | ((TARGET_P8_VECTOR) ? RS6000_BTM_P8_VECTOR : 0) > | ((TARGET_CRYPTO) ? RS6000_BTM_CRYPTO : 0) > | ((TARGET_HTM) ? RS6000_BTM_HTM : 0) > - | ((TARGET_DFP) ? RS6000_BTM_DFP : 0)); > + | ((TARGET_DFP) ? RS6000_BTM_DFP : 0) > + | ((TARGET_HARD_FLOAT) ? RS6000_BTM_HARD_FLOAT: 0)); This is missing a space between RS6000_BTM_HARD_FLOAT and ":". > } This is okay on trunk and all branches. Thanks David
On Sun, 2014-05-04 at 17:39 -0400, David Edelsohn wrote: > On Fri, May 2, 2014 at 7:34 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > > @@ -2517,6 +2518,7 @@ extern int frame_pointer_needed; > > #define RS6000_BTM_POPCNTD MASK_POPCNTD /* Target supports ISA 2.06. */ > > #define RS6000_BTM_CELL MASK_FPRND /* Target is cell powerpc. */ > > #define RS6000_BTM_DFP MASK_DFP /* Decimal floating point. */ > > +#define RS6000_BTM_HARD_FLOAT MASK_SOFT_FLOAT /* Hardware floating point. */ > > It's a little confusing that the use reverses the polarity of the mask meaning. Agreed, but since there is no MASK_HARD_FLOAT, we're stuck with that. However, the use of the MASK here is only used to define which bit within the builtin bitmask corresponds to hard float. The actual setting of the builtin bitmask occurs below in rs6000_builtin_mask_calculate and there we use the BTM mask we defined above. > > Index: gcc/config/rs6000/rs6000.c > > =================================================================== > > --- gcc/config/rs6000/rs6000.c (revision 209990) > > +++ gcc/config/rs6000/rs6000.c (working copy) > > @@ -3039,7 +3039,8 @@ rs6000_builtin_mask_calculate (void) > > | ((TARGET_P8_VECTOR) ? RS6000_BTM_P8_VECTOR : 0) > > | ((TARGET_CRYPTO) ? RS6000_BTM_CRYPTO : 0) > > | ((TARGET_HTM) ? RS6000_BTM_HTM : 0) > > - | ((TARGET_DFP) ? RS6000_BTM_DFP : 0)); > > + | ((TARGET_DFP) ? RS6000_BTM_DFP : 0) > > + | ((TARGET_HARD_FLOAT) ? RS6000_BTM_HARD_FLOAT: 0)); > > This is missing a space between RS6000_BTM_HARD_FLOAT and ":". Fixed. > This is okay on trunk and all branches. Thanks, committed to trunk as revision 210054, 4.9 as revision 210055 and to 4.8 as revision 210056. Peter
Index: gcc/config/rs6000/rs6000.h =================================================================== --- gcc/config/rs6000/rs6000.h (revision 209990) +++ gcc/config/rs6000/rs6000.h (working copy) @@ -624,7 +624,8 @@ extern int rs6000_vector_align[]; || TARGET_CMPB /* ISA 2.05 */ \ || TARGET_POPCNTD /* ISA 2.06 */ \ || TARGET_ALTIVEC \ - || TARGET_VSX))) + || TARGET_VSX \ + || TARGET_HARD_FLOAT))) /* E500 cores only support plain "sync", not lwsync. */ #define TARGET_NO_LWSYNC (rs6000_cpu == PROCESSOR_PPC8540 \ @@ -2517,6 +2518,7 @@ extern int frame_pointer_needed; #define RS6000_BTM_POPCNTD MASK_POPCNTD /* Target supports ISA 2.06. */ #define RS6000_BTM_CELL MASK_FPRND /* Target is cell powerpc. */ #define RS6000_BTM_DFP MASK_DFP /* Decimal floating point. */ +#define RS6000_BTM_HARD_FLOAT MASK_SOFT_FLOAT /* Hardware floating point. */ #define RS6000_BTM_COMMON (RS6000_BTM_ALTIVEC \ | RS6000_BTM_VSX \ @@ -2529,7 +2531,8 @@ extern int frame_pointer_needed; | RS6000_BTM_HTM \ | RS6000_BTM_POPCNTD \ | RS6000_BTM_CELL \ - | RS6000_BTM_DFP) + | RS6000_BTM_DFP \ + | RS6000_BTM_HARD_FLOAT) /* Define builtin enum index. */ Index: gcc/config/rs6000/rs6000-builtin.def =================================================================== --- gcc/config/rs6000/rs6000-builtin.def (revision 209990) +++ gcc/config/rs6000/rs6000-builtin.def (working copy) @@ -626,7 +626,7 @@ #define BU_MISC_1(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_2 (MISC_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ - RS6000_BTM_ALWAYS, /* MASK */ \ + RS6000_BTM_HARD_FLOAT, /* MASK */ \ (RS6000_BTC_ ## ATTR /* ATTR */ \ | RS6000_BTC_UNARY), \ CODE_FOR_ ## ICODE) /* ICODE */ @@ -634,7 +634,7 @@ #define BU_MISC_2(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_2 (MISC_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ - RS6000_BTM_ALWAYS, /* MASK */ \ + RS6000_BTM_HARD_FLOAT, /* MASK */ \ (RS6000_BTC_ ## ATTR /* ATTR */ \ | RS6000_BTC_BINARY), \ CODE_FOR_ ## ICODE) /* ICODE */ Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 209990) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -3039,7 +3039,8 @@ rs6000_builtin_mask_calculate (void) | ((TARGET_P8_VECTOR) ? RS6000_BTM_P8_VECTOR : 0) | ((TARGET_CRYPTO) ? RS6000_BTM_CRYPTO : 0) | ((TARGET_HTM) ? RS6000_BTM_HTM : 0) - | ((TARGET_DFP) ? RS6000_BTM_DFP : 0)); + | ((TARGET_DFP) ? RS6000_BTM_DFP : 0) + | ((TARGET_HARD_FLOAT) ? RS6000_BTM_HARD_FLOAT: 0)); } /* Override command line options. Mostly we process the processor type and @@ -3396,6 +3397,13 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_VSX_TIMODE; } + if (TARGET_DFP && !TARGET_HARD_FLOAT) + { + if (rs6000_isa_flags_explicit & OPTION_MASK_DFP) + error ("-mhard-dfp requires -mhard-float"); + rs6000_isa_flags &= ~OPTION_MASK_DFP; + } + /* The quad memory instructions only works in 64-bit mode. In 32-bit mode, silently turn off quad memory mode. */ if ((TARGET_QUAD_MEMORY || TARGET_QUAD_MEMORY_ATOMIC) && !TARGET_POWERPC64) @@ -13559,6 +13567,8 @@ rs6000_invalid_builtin (enum rs6000_buil error ("Builtin function %s requires the -mhard-dfp option", name); else if ((fnmask & RS6000_BTM_P8_VECTOR) != 0) error ("Builtin function %s requires the -mpower8-vector option", name); + else if ((fnmask & RS6000_BTM_HARD_FLOAT) != 0) + error ("Builtin function %s requires the -mhard-float option", name); else error ("Builtin function %s is not supported with the current options", name); @@ -13747,7 +13757,10 @@ rs6000_expand_builtin (tree exp, rtx tar return ret; } - gcc_assert (TARGET_ALTIVEC || TARGET_VSX || TARGET_SPE || TARGET_PAIRED_FLOAT); + unsigned attr = rs6000_builtin_info[uns_fcode].attr & RS6000_BTC_TYPE_MASK; + gcc_assert (attr == RS6000_BTC_UNARY + || attr == RS6000_BTC_BINARY + || attr == RS6000_BTC_TERNARY); /* Handle simple unary operations. */ d = bdesc_1arg; @@ -31302,6 +31315,7 @@ static struct rs6000_opt_mask const rs60 { "crypto", RS6000_BTM_CRYPTO, false, false }, { "htm", RS6000_BTM_HTM, false, false }, { "hard-dfp", RS6000_BTM_DFP, false, false }, + { "hard-float", RS6000_BTM_HARD_FLOAT, false, false }, }; /* Option variables that we want to support inside attribute((target)) and Index: gcc/testsuite/gcc.target/powerpc/pack02.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pack02.c (revision 209990) +++ gcc/testsuite/gcc.target/powerpc/pack02.c (working copy) @@ -1,8 +1,8 @@ /* { dg-do run { target { powerpc*-*-linux* } } } */ /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ /* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */ -/* { dg-require-effective-target vsx_hw } */ -/* { dg-options "-O2" } */ +/* { dg-require-effective-target powerpc_fprs } */ +/* { dg-options "-O2 -mhard-float" } */ #include <stddef.h> #include <stdlib.h> Index: gcc/testsuite/gcc.target/powerpc/pack03.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pack03.c (revision 209990) +++ gcc/testsuite/gcc.target/powerpc/pack03.c (working copy) @@ -1,8 +1,8 @@ /* { dg-do run { target { powerpc*-*-linux* } } } */ /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ /* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */ -/* { dg-require-effective-target vsx_hw } */ -/* { dg-options "-O2" } */ +/* { dg-require-effective-target dfprt } */ +/* { dg-options "-O2 -mhard-dfp" } */ #include <stddef.h> #include <stdlib.h>