diff mbox series

Fix mismatched precisions in tree arithmetic

Message ID 87o9pqx2vj.fsf@linaro.org
State New
Headers show
Series Fix mismatched precisions in tree arithmetic | expand

Commit Message

Richard Sandiford Oct. 1, 2017, 4:13 p.m. UTC
The tree wi:: decompose routine wasn't asserting that the requested
precision matched the tree's precision.  This could make a difference
for unsigned trees that are exactly N HWIs wide and that have the upper
bit set, since we then need an extra zero HWI when extending it to wider
precisions (as for wi::to_widest).

This patch adds the assert and fixes the fallout shown by the testsuite.
Go seems to be unaffected.

Other fixed-precision decompose routines already had an assert.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
Also tested by comparing the testsuite assembly output on at least one
target per CPU directory.  OK to install?

Richard


2017-10-01  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* tree.h (wi::int_traits <const_tree>::decompose): Assert that the
	requested precision matches the type's.
	* calls.c (alloc_max_size): Calculate the new candidate size as
	a widest_int and use wi::to_widest when comparing it with the
	current candidate size.
	* gimple-ssa-warn-alloca.c (pass_walloca::execute): Compare with
	zero rather than integer_zero_node.
	* match.pd: Check for a no-op conversion before using wi::add
	rather than after.  Use tree_to_uhwi when summing small shift
	counts into an unsigned int.

gcc/c-family/
	* c-warn.c (warn_tautological_bitwise_comparison): Use wi::to_widest
	when combining the original unconverted comparison operands.

gcc/cp/
	* constexpr.c (cxx_eval_store_expression): Use wi::to_widest
	when comparing the array bounds with an ARRAY_REF index.

gcc/ada/
	* gcc-interface/decl.c (annotate_value): Use wi::to_widest when
	handling the form (plus/mult (convert @0) @1).

Comments

Richard Biener Oct. 2, 2017, 9:17 a.m. UTC | #1
On Sun, Oct 1, 2017 at 6:13 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> The tree wi:: decompose routine wasn't asserting that the requested

> precision matched the tree's precision.  This could make a difference

> for unsigned trees that are exactly N HWIs wide and that have the upper

> bit set, since we then need an extra zero HWI when extending it to wider

> precisions (as for wi::to_widest).

>

> This patch adds the assert and fixes the fallout shown by the testsuite.

> Go seems to be unaffected.

>

> Other fixed-precision decompose routines already had an assert.

>

> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.

> Also tested by comparing the testsuite assembly output on at least one

> target per CPU directory.  OK to install?


Ok.

Thanks,
Richard.

> Richard

>

>

> 2017-10-01  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/

>         * tree.h (wi::int_traits <const_tree>::decompose): Assert that the

>         requested precision matches the type's.

>         * calls.c (alloc_max_size): Calculate the new candidate size as

>         a widest_int and use wi::to_widest when comparing it with the

>         current candidate size.

>         * gimple-ssa-warn-alloca.c (pass_walloca::execute): Compare with

>         zero rather than integer_zero_node.

>         * match.pd: Check for a no-op conversion before using wi::add

>         rather than after.  Use tree_to_uhwi when summing small shift

>         counts into an unsigned int.

>

> gcc/c-family/

>         * c-warn.c (warn_tautological_bitwise_comparison): Use wi::to_widest

>         when combining the original unconverted comparison operands.

>

> gcc/cp/

>         * constexpr.c (cxx_eval_store_expression): Use wi::to_widest

>         when comparing the array bounds with an ARRAY_REF index.

>

> gcc/ada/

>         * gcc-interface/decl.c (annotate_value): Use wi::to_widest when

>         handling the form (plus/mult (convert @0) @1).

>

> Index: gcc/tree.h

> ===================================================================

> --- gcc/tree.h  2017-09-25 13:33:39.989814299 +0100

> +++ gcc/tree.h  2017-10-01 17:08:55.827120507 +0100

> @@ -5176,6 +5176,7 @@ wi::int_traits <const_tree>::get_precisi

>  wi::int_traits <const_tree>::decompose (HOST_WIDE_INT *,

>                                         unsigned int precision, const_tree x)

>  {

> +  gcc_checking_assert (precision == TYPE_PRECISION (TREE_TYPE (x)));

>    return wi::storage_ref (&TREE_INT_CST_ELT (x, 0), TREE_INT_CST_NUNITS (x),

>                           precision);

>  }

> Index: gcc/calls.c

> ===================================================================

> --- gcc/calls.c 2017-09-21 12:13:48.336399601 +0100

> +++ gcc/calls.c 2017-10-01 17:08:55.825112782 +0100

> @@ -1252,9 +1252,8 @@ alloc_max_size (void)

>

>               if (unit)

>                 {

> -                 wide_int w = wi::uhwi (limit, HOST_BITS_PER_WIDE_INT + 64);

> -                 w *= unit;

> -                 if (wi::ltu_p (w, alloc_object_size_limit))

> +                 widest_int w = wi::mul (limit, unit);

> +                 if (w < wi::to_widest (alloc_object_size_limit))

>                     alloc_object_size_limit = wide_int_to_tree (ssizetype, w);

>                 }

>             }

> Index: gcc/gimple-ssa-warn-alloca.c

> ===================================================================

> --- gcc/gimple-ssa-warn-alloca.c        2017-03-28 16:19:28.000000000 +0100

> +++ gcc/gimple-ssa-warn-alloca.c        2017-10-01 17:08:55.826116645 +0100

> @@ -491,7 +491,7 @@ pass_walloca::execute (function *fun)

>                               is_vla ? G_("argument to variable-length array "

>                                           "may be too large")

>                               : G_("argument to %<alloca%> may be too large"))

> -                 && t.limit != integer_zero_node)

> +                 && t.limit != 0)

>                 {

>                   print_decu (t.limit, buff);

>                   inform (loc, G_("limit is %u bytes, but argument "

> @@ -504,7 +504,7 @@ pass_walloca::execute (function *fun)

>                               is_vla ? G_("argument to variable-length array "

>                                           "is too large")

>                               : G_("argument to %<alloca%> is too large"))

> -                 && t.limit != integer_zero_node)

> +                 && t.limit != 0)

>                 {

>                   print_decu (t.limit, buff);

>                   inform (loc, G_("limit is %u bytes, but argument is %s"),

> Index: gcc/match.pd

> ===================================================================

> --- gcc/match.pd        2017-09-25 13:57:11.698158636 +0100

> +++ gcc/match.pd        2017-10-01 17:08:55.827120507 +0100

> @@ -358,8 +358,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>    (div (convert? (bit_and @0 INTEGER_CST@1)) INTEGER_CST@2)

>    (if (integer_pow2p (@2)

>         && tree_int_cst_sgn (@2) > 0

> -       && wi::add (@2, @1) == 0

> -       && tree_nop_conversion_p (type, TREE_TYPE (@0)))

> +       && tree_nop_conversion_p (type, TREE_TYPE (@0))

> +       && wi::add (@2, @1) == 0)

>     (rshift (convert @0) { build_int_cst (integer_type_node,

>                                          wi::exact_log2 (@2)); }))))

>

> @@ -1871,7 +1871,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>          && wi::lt_p (@1, prec, TYPE_SIGN (TREE_TYPE (@1)))

>          && wi::ge_p (@2, 0, TYPE_SIGN (TREE_TYPE (@2)))

>         && wi::lt_p (@2, prec, TYPE_SIGN (TREE_TYPE (@2))))

> -    (with { unsigned int low = wi::add (@1, @2).to_uhwi (); }

> +    (with { unsigned int low = (tree_to_uhwi (@1)

> +                               + tree_to_uhwi (@2)); }

>       /* Deal with a OP (c1 + c2) being undefined but (a OP c1) OP c2

>          being well defined.  */

>       (if (low >= prec)

> Index: gcc/c-family/c-warn.c

> ===================================================================

> --- gcc/c-family/c-warn.c       2017-09-05 20:55:56.402037612 +0100

> +++ gcc/c-family/c-warn.c       2017-10-01 17:08:55.824108920 +0100

> @@ -355,15 +355,17 @@ warn_tautological_bitwise_comparison (lo

>    else

>      return;

>

> -  wide_int res;

> +  /* Note that the two operands are from before the usual integer

> +     conversions, so their types might not be the same.  */

> +  widest_int res;

>    if (TREE_CODE (bitop) == BIT_AND_EXPR)

> -    res = wi::bit_and (bitopcst, cst);

> +    res = wi::to_widest (bitopcst) & wi::to_widest (cst);

>    else

> -    res = wi::bit_or (bitopcst, cst);

> +    res = wi::to_widest (bitopcst) | wi::to_widest (cst);

>

>    /* For BIT_AND only warn if (CST2 & CST1) != CST1, and

>       for BIT_OR only if (CST2 | CST1) != CST1.  */

> -  if (res == cst)

> +  if (res == wi::to_widest (cst))

>      return;

>

>    if (code == EQ_EXPR)

> Index: gcc/cp/constexpr.c

> ===================================================================

> --- gcc/cp/constexpr.c  2017-09-16 21:38:21.073925724 +0100

> +++ gcc/cp/constexpr.c  2017-10-01 17:08:55.826116645 +0100

> @@ -3379,7 +3379,7 @@ cxx_eval_store_expression (const constex

>           VERIFY_CONSTANT (nelts);

>           gcc_assert (TREE_CODE (nelts) == INTEGER_CST

>                       && TREE_CODE (TREE_OPERAND (probe, 1)) == INTEGER_CST);

> -         if (wi::eq_p (TREE_OPERAND (probe, 1), nelts))

> +         if (wi::to_widest (TREE_OPERAND (probe, 1)) == wi::to_widest (nelts))

>             {

>               diag_array_subscript (ctx, ary, TREE_OPERAND (probe, 1));

>               *non_constant_p = true;

> Index: gcc/ada/gcc-interface/decl.c

> ===================================================================

> --- gcc/ada/gcc-interface/decl.c        2017-09-11 17:10:55.549140040 +0100

> +++ gcc/ada/gcc-interface/decl.c        2017-10-01 17:08:55.823105057 +0100

> @@ -8153,11 +8153,13 @@ annotate_value (tree gnu_size)

>             {

>               tree inner_op_op1 = TREE_OPERAND (inner_op, 1);

>               tree gnu_size_op1 = TREE_OPERAND (gnu_size, 1);

> -             wide_int op1;

> +             widest_int op1;

>               if (TREE_CODE (gnu_size) == MULT_EXPR)

> -               op1 = wi::mul (inner_op_op1, gnu_size_op1);

> +               op1 = (wi::to_widest (inner_op_op1)

> +                      * wi::to_widest (gnu_size_op1));

>               else

> -               op1 = wi::add (inner_op_op1, gnu_size_op1);

> +               op1 = (wi::to_widest (inner_op_op1)

> +                      + wi::to_widest (gnu_size_op1));

>               ops[1] = UI_From_gnu (wide_int_to_tree (sizetype, op1));

>               ops[0] = annotate_value (TREE_OPERAND (inner_op, 0));

>             }
diff mbox series

Patch

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	2017-09-25 13:33:39.989814299 +0100
+++ gcc/tree.h	2017-10-01 17:08:55.827120507 +0100
@@ -5176,6 +5176,7 @@  wi::int_traits <const_tree>::get_precisi
 wi::int_traits <const_tree>::decompose (HOST_WIDE_INT *,
 					unsigned int precision, const_tree x)
 {
+  gcc_checking_assert (precision == TYPE_PRECISION (TREE_TYPE (x)));
   return wi::storage_ref (&TREE_INT_CST_ELT (x, 0), TREE_INT_CST_NUNITS (x),
 			  precision);
 }
Index: gcc/calls.c
===================================================================
--- gcc/calls.c	2017-09-21 12:13:48.336399601 +0100
+++ gcc/calls.c	2017-10-01 17:08:55.825112782 +0100
@@ -1252,9 +1252,8 @@  alloc_max_size (void)
 
 	      if (unit)
 		{
-		  wide_int w = wi::uhwi (limit, HOST_BITS_PER_WIDE_INT + 64);
-		  w *= unit;
-		  if (wi::ltu_p (w, alloc_object_size_limit))
+		  widest_int w = wi::mul (limit, unit);
+		  if (w < wi::to_widest (alloc_object_size_limit))
 		    alloc_object_size_limit = wide_int_to_tree (ssizetype, w);
 		}
 	    }
Index: gcc/gimple-ssa-warn-alloca.c
===================================================================
--- gcc/gimple-ssa-warn-alloca.c	2017-03-28 16:19:28.000000000 +0100
+++ gcc/gimple-ssa-warn-alloca.c	2017-10-01 17:08:55.826116645 +0100
@@ -491,7 +491,7 @@  pass_walloca::execute (function *fun)
 			      is_vla ? G_("argument to variable-length array "
 					  "may be too large")
 			      : G_("argument to %<alloca%> may be too large"))
-		  && t.limit != integer_zero_node)
+		  && t.limit != 0)
 		{
 		  print_decu (t.limit, buff);
 		  inform (loc, G_("limit is %u bytes, but argument "
@@ -504,7 +504,7 @@  pass_walloca::execute (function *fun)
 			      is_vla ? G_("argument to variable-length array "
 					  "is too large")
 			      : G_("argument to %<alloca%> is too large"))
-		  && t.limit != integer_zero_node)
+		  && t.limit != 0)
 		{
 		  print_decu (t.limit, buff);
 		  inform (loc, G_("limit is %u bytes, but argument is %s"),
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	2017-09-25 13:57:11.698158636 +0100
+++ gcc/match.pd	2017-10-01 17:08:55.827120507 +0100
@@ -358,8 +358,8 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (div (convert? (bit_and @0 INTEGER_CST@1)) INTEGER_CST@2)
   (if (integer_pow2p (@2)
        && tree_int_cst_sgn (@2) > 0
-       && wi::add (@2, @1) == 0
-       && tree_nop_conversion_p (type, TREE_TYPE (@0)))
+       && tree_nop_conversion_p (type, TREE_TYPE (@0))
+       && wi::add (@2, @1) == 0)
    (rshift (convert @0) { build_int_cst (integer_type_node,
 					 wi::exact_log2 (@2)); }))))
 
@@ -1871,7 +1871,8 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
         && wi::lt_p (@1, prec, TYPE_SIGN (TREE_TYPE (@1)))
         && wi::ge_p (@2, 0, TYPE_SIGN (TREE_TYPE (@2)))
 	&& wi::lt_p (@2, prec, TYPE_SIGN (TREE_TYPE (@2))))
-    (with { unsigned int low = wi::add (@1, @2).to_uhwi (); }
+    (with { unsigned int low = (tree_to_uhwi (@1)
+				+ tree_to_uhwi (@2)); }
      /* Deal with a OP (c1 + c2) being undefined but (a OP c1) OP c2
         being well defined.  */
      (if (low >= prec)
Index: gcc/c-family/c-warn.c
===================================================================
--- gcc/c-family/c-warn.c	2017-09-05 20:55:56.402037612 +0100
+++ gcc/c-family/c-warn.c	2017-10-01 17:08:55.824108920 +0100
@@ -355,15 +355,17 @@  warn_tautological_bitwise_comparison (lo
   else
     return;
 
-  wide_int res;
+  /* Note that the two operands are from before the usual integer
+     conversions, so their types might not be the same.  */
+  widest_int res;
   if (TREE_CODE (bitop) == BIT_AND_EXPR)
-    res = wi::bit_and (bitopcst, cst);
+    res = wi::to_widest (bitopcst) & wi::to_widest (cst);
   else
-    res = wi::bit_or (bitopcst, cst);
+    res = wi::to_widest (bitopcst) | wi::to_widest (cst);
 
   /* For BIT_AND only warn if (CST2 & CST1) != CST1, and
      for BIT_OR only if (CST2 | CST1) != CST1.  */
-  if (res == cst)
+  if (res == wi::to_widest (cst))
     return;
 
   if (code == EQ_EXPR)
Index: gcc/cp/constexpr.c
===================================================================
--- gcc/cp/constexpr.c	2017-09-16 21:38:21.073925724 +0100
+++ gcc/cp/constexpr.c	2017-10-01 17:08:55.826116645 +0100
@@ -3379,7 +3379,7 @@  cxx_eval_store_expression (const constex
 	  VERIFY_CONSTANT (nelts);
 	  gcc_assert (TREE_CODE (nelts) == INTEGER_CST
 		      && TREE_CODE (TREE_OPERAND (probe, 1)) == INTEGER_CST);
-	  if (wi::eq_p (TREE_OPERAND (probe, 1), nelts))
+	  if (wi::to_widest (TREE_OPERAND (probe, 1)) == wi::to_widest (nelts))
 	    {
 	      diag_array_subscript (ctx, ary, TREE_OPERAND (probe, 1));
 	      *non_constant_p = true;
Index: gcc/ada/gcc-interface/decl.c
===================================================================
--- gcc/ada/gcc-interface/decl.c	2017-09-11 17:10:55.549140040 +0100
+++ gcc/ada/gcc-interface/decl.c	2017-10-01 17:08:55.823105057 +0100
@@ -8153,11 +8153,13 @@  annotate_value (tree gnu_size)
 	    {
 	      tree inner_op_op1 = TREE_OPERAND (inner_op, 1);
 	      tree gnu_size_op1 = TREE_OPERAND (gnu_size, 1);
-	      wide_int op1;
+	      widest_int op1;
 	      if (TREE_CODE (gnu_size) == MULT_EXPR)
-		op1 = wi::mul (inner_op_op1, gnu_size_op1);
+		op1 = (wi::to_widest (inner_op_op1)
+		       * wi::to_widest (gnu_size_op1));
 	      else
-		op1 = wi::add (inner_op_op1, gnu_size_op1);
+		op1 = (wi::to_widest (inner_op_op1)
+		       + wi::to_widest (gnu_size_op1));
 	      ops[1] = UI_From_gnu (wide_int_to_tree (sizetype, op1));
 	      ops[0] = annotate_value (TREE_OPERAND (inner_op, 0));
 	    }