Re: [PATCH, RS6000] Fix __builtin_{pack,unpack}_longdouble and __builtin_{pack,unpack}_dec128 builtins

[David Edelsohn <dje dot gcc at gmail dot com>]

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