Re: [PATCH] Partially revert undesirable widest literal type changes from 1999 (PR middle-end/79788, PR middle-end/80375)

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Apr 10, 2017 at 10:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The http://gcc.gnu.org/ml/gcc-patches/1999-07/msg00031.html change
> appears to be IMHO highly undesirable loophole around the lack of
> TImode support on 32-bit targets.  Those targets return
> false from targetm.scalar_mode_supported_p (TImode) for a reason,
> while for some arithmetics operation perhaps the middle-end manages
> to emit something that doesn't ICE, as the following testcases shows
> there are various other cases which just ICE.
> While the 32-bit targets don't support __int128/__int128_t/__uint128_t,
> because of the above change one can use instead
> __typeof (1234567891234567891234567891234567812) or similar constructs
> to get variables of that type and similarly expressions can still have
> the 128-bit type, but as can be seen we don't really support those.
>
> Thus, this patch reverts the effect of the above patch for 32-bit targets,
> basically the type of the oversized literals is now __int128_t/__uint128_t
> on 64-bit targets as before, but only long long/unsigned long long on
> 32-bit targets.  While this is an ABI change, it affects only code on which
> we emit pedwarn, so I'd think it is unlikely to be used in the wild.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok if Joseph doesn't have a better idea.

It may be still worth noting that to the caveats section of changes.html.

Richard.

> 2017-04-10  Jakub Jelinek  <jakub@redhat.com>
>
>         PR middle-end/79788
>         PR middle-end/80375
>         * c-common.c (c_common_type_for_mode): Don't handle
>         widest_*_literal_type_node here.
>         c_common_signed_or_unsigned_type): Likewise.
>         (c_common_nodes_and_builtins): Set widest_*_literal_type_node
>         to *intTI_type_node or *intDI_type_node depending on whether
>         TImode is supported by the target or not.
>
>         * gcc.dg/pr79788-1.c: New test.
>         * gcc.dg/pr79788-2.c: New test.
>
> --- gcc/c-family/c-common.c.jj  2017-03-31 08:39:08.000000000 +0200
> +++ gcc/c-family/c-common.c     2017-04-10 13:29:10.691179060 +0200
> @@ -2179,10 +2179,6 @@ c_common_type_for_mode (machine_mode mod
>        return (unsignedp ? int_n_trees[i].unsigned_type
>               : int_n_trees[i].signed_type);
>
> -  if (mode == TYPE_MODE (widest_integer_literal_type_node))
> -    return unsignedp ? widest_unsigned_literal_type_node
> -                    : widest_integer_literal_type_node;
> -
>    if (mode == QImode)
>      return unsignedp ? unsigned_intQI_type_node : intQI_type_node;
>
> @@ -2412,8 +2408,6 @@ c_common_signed_or_unsigned_type (int un
>        return (unsignedp ? int_n_trees[i].unsigned_type
>               : int_n_trees[i].signed_type);
>
> -  if (type1 == widest_integer_literal_type_node || type1 == widest_unsigned_literal_type_node)
> -    return unsignedp ? widest_unsigned_literal_type_node : widest_integer_literal_type_node;
>  #if HOST_BITS_PER_WIDE_INT >= 64
>    if (type1 == intTI_type_node || type1 == unsigned_intTI_type_node)
>      return unsignedp ? unsigned_intTI_type_node : intTI_type_node;
> @@ -2534,10 +2528,6 @@ c_common_signed_or_unsigned_type (int un
>        return (unsignedp ? int_n_trees[i].unsigned_type
>               : int_n_trees[i].signed_type);
>
> -  if (TYPE_OK (widest_integer_literal_type_node))
> -    return (unsignedp ? widest_unsigned_literal_type_node
> -           : widest_integer_literal_type_node);
> -
>  #if HOST_BITS_PER_WIDE_INT >= 64
>    if (TYPE_OK (intTI_type_node))
>      return unsignedp ? unsigned_intTI_type_node : intTI_type_node;
> @@ -4164,17 +4154,16 @@ c_common_nodes_and_builtins (void)
>  #endif
>
>    /* Create the widest literal types.  */
> -  widest_integer_literal_type_node
> -    = make_signed_type (HOST_BITS_PER_WIDE_INT * 2);
> -  lang_hooks.decls.pushdecl (build_decl (UNKNOWN_LOCATION,
> -                                        TYPE_DECL, NULL_TREE,
> -                                        widest_integer_literal_type_node));
> -
> -  widest_unsigned_literal_type_node
> -    = make_unsigned_type (HOST_BITS_PER_WIDE_INT * 2);
> -  lang_hooks.decls.pushdecl (build_decl (UNKNOWN_LOCATION,
> -                                        TYPE_DECL, NULL_TREE,
> -                                        widest_unsigned_literal_type_node));
> +  if (targetm.scalar_mode_supported_p (TImode))
> +    {
> +      widest_integer_literal_type_node = intTI_type_node;
> +      widest_unsigned_literal_type_node = unsigned_intTI_type_node;
> +    }
> +  else
> +    {
> +      widest_integer_literal_type_node = intDI_type_node;
> +      widest_unsigned_literal_type_node = unsigned_intDI_type_node;
> +    }
>
>    signed_size_type_node = c_common_signed_type (size_type_node);
>
> --- gcc/testsuite/gcc.dg/pr79788-1.c.jj 2017-04-10 13:58:33.399248477 +0200
> +++ gcc/testsuite/gcc.dg/pr79788-1.c    2017-04-10 13:59:17.980699210 +0200
> @@ -0,0 +1,11 @@
> +/* PR middle-end/79788 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +long long
> +foo (long long x, long long y)
> +{
> +  if (y > 1234567891234567891234567891234567812 / x)   /* { dg-warning "integer constant is too large for its type" } */
> +    return x;
> +  return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr79788-2.c.jj 2017-04-10 13:58:37.171202004 +0200
> +++ gcc/testsuite/gcc.dg/pr79788-2.c    2017-04-10 13:57:33.000000000 +0200
> @@ -0,0 +1,11 @@
> +/* PR middle-end/79788 */
> +/* { dg-do compile } */
> +/* { dg-options "-ftrapv" } */
> +
> +void bar (void);
> +void
> +foo (long long int p, long long int q)
> +{
> +  if (p >= 1234567891234567891234567891234567812 + q)  /* { dg-warning "integer constant is too large for its type" } */
> +    bar ();
> +}
>
>         Jakub