[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

Richard Biener  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2018-11-30
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org
 Ever confirmed|0   |1

--- Comment #1 from Richard Biener  ---
-fstrict-enums.  By inspection ranges_from_anti_range falls foul of our
incomplete honoring of TYPE_MIN/MAX_VALUE.  Don't have a cross ready
to test the following patch but it looks "obvious" ...

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 266657)
+++ gcc/tree-vrp.c  (working copy)
@@ -1249,14 +1249,14 @@ ranges_from_anti_range (const value_rang
   || !vrp_val_max (type))
 return false;

-  if (!vrp_val_is_min (ar->min ()))
-*vr0 = value_range (VR_RANGE,
-   vrp_val_min (type),
-   wide_int_to_tree (type, wi::to_wide (ar->min ()) - 1));
-  if (!vrp_val_is_max (ar->max ()))
-*vr1 = value_range (VR_RANGE,
-   wide_int_to_tree (type, wi::to_wide (ar->max ()) + 1),
-   vrp_val_max (type));
+  if (tree_int_cst_lt (vrp_val_min (type), ar->min ()))
+vr0->set (VR_RANGE,
+ vrp_val_min (type),
+ wide_int_to_tree (type, wi::to_wide (ar->min ()) - 1));
+  if (tree_int_cst_lt (ar->max (), vrp_val_max (type)))
+vr1->set (VR_RANGE,
+ wide_int_to_tree (type, wi::to_wide (ar->max ()) + 1),
+ vrp_val_max (type));
   if (vr0->undefined_p ())
 {
   *vr0 = *vr1;

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread asolokha at gmx dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #2 from Arseny Solokha  ---
Indeed it fixes the issue at hand. I won't be able to perform full regtest
until the beginning of next week, though.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Richard Biener  ---
Fixed.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #4 from Richard Biener  ---
Author: rguenth
Date: Fri Nov 30 10:37:39 2018
New Revision: 266659

URL: https://gcc.gnu.org/viewcvs?rev=266659&root=gcc&view=rev
Log:
2018-11-30  Richard Biener  

PR tree-optimization/88274
* tree-vrp.c (ranges_from_anti_range): Fix handling of
TYPE_MIN/MAX_VALUE.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/tree-vrp.c

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #5 from Jakub Jelinek  ---
With additional -mbranch-cost=1 the gimple dumps are identical on x86_64 with
-m32 until reassoc1, which decides differently for some reason - diff from
powerpc64 to x86_64:
-Optimizing range tests _7 -[3, 3] and -[5, 5]
- into (_7 + 4294967293 & 4294967293) != 0
-Rank for _38 is 458754
-Width = 1 was chosen for reassociation
-Width = 1 was chosen for reassociation
-Removing basic block 7
...

I guess one bug here is in reassoc pass, if we have these weird types that
don't have precision of the corresponding mode, we should have probably cast it
to some normal integral type before doing the + 429467293 etc. on it, that is
probably UB on such an IL.

The other issue is that the vrp code shouldn't ICE on code like:
  reload_type _7;
  reload_type _38;
  reload_type _39;

  _7 = MEM[base: _42, offset: 16B];
  _38 = _7 + 4294967293;
  _39 = _38 & 4294967293;
where reload_type is unsigned type with range [0, 15] and precision 4.
I haven't managed to create a testcase where it would trigger through invalid
code though so far.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #6 from rguenther at suse dot de  ---
On Fri, 30 Nov 2018, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274
> 
> --- Comment #5 from Jakub Jelinek  ---
> With additional -mbranch-cost=1 the gimple dumps are identical on x86_64 with
> -m32 until reassoc1, which decides differently for some reason - diff from
> powerpc64 to x86_64:
> -Optimizing range tests _7 -[3, 3] and -[5, 5]
> - into (_7 + 4294967293 & 4294967293) != 0
> -Rank for _38 is 458754
> -Width = 1 was chosen for reassociation
> -Width = 1 was chosen for reassociation
> -Removing basic block 7
> ...
> 
> I guess one bug here is in reassoc pass, if we have these weird types that
> don't have precision of the corresponding mode, we should have probably cast 
> it
> to some normal integral type before doing the + 429467293 etc. on it, that is
> probably UB on such an IL.
> 
> The other issue is that the vrp code shouldn't ICE on code like:
>   reload_type _7;
>   reload_type _38;
>   reload_type _39;
> 
>   _7 = MEM[base: _42, offset: 16B];
>   _38 = _7 + 4294967293;
>   _39 = _38 & 4294967293;
> where reload_type is unsigned type with range [0, 15] and precision 4.
> I haven't managed to create a testcase where it would trigger through invalid
> code though so far.

I think those min/max values not matching the precision are only an
optimization hint.  But yes, if a pass creates out-of-range values
then it has to use a different type for those.

Or we simply go away with honoring those hints in VRP at all...

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #7 from Jakub Jelinek  ---
The regression started with r265241 BTW.

Here is a reassoc patch I wrote that also fixes this ICE:
--- gcc/tree-ssa-reassoc.c.jj   2018-10-23 10:13:25.278875175 +0200
+++ gcc/tree-ssa-reassoc.c  2018-11-30 11:13:37.232393154 +0100
@@ -2537,8 +2537,23 @@ optimize_range_tests_xor (enum tree_code
   if (!tree_int_cst_equal (lowxor, highxor))
 return false;

+  exp = rangei->exp;
+  scalar_int_mode mode = as_a  (TYPE_MODE (type));
+  int prec = GET_MODE_PRECISION (mode);
+  if (TYPE_PRECISION (type) < prec
+  || (wi::to_wide (TYPE_MIN_VALUE (type))
+ != wi::min_value (prec, TYPE_SIGN (type)))
+  || (wi::to_wide (TYPE_MAX_VALUE (type))
+ != wi::max_value (prec, TYPE_SIGN (type
+{
+  type = build_nonstandard_integer_type (prec, TYPE_UNSIGNED (type));
+  exp = fold_convert (type, exp);
+  lowxor = fold_convert (type, lowxor);
+  lowi = fold_convert (type, lowi);
+  highi = fold_convert (type, highi);
+}
   tem = fold_build1 (BIT_NOT_EXPR, type, lowxor);
-  exp = fold_build2 (BIT_AND_EXPR, type, rangei->exp, tem);
+  exp = fold_build2 (BIT_AND_EXPR, type, exp, tem);
   lowj = fold_build2 (BIT_AND_EXPR, type, lowi, tem);
   highj = fold_build2 (BIT_AND_EXPR, type, highi, tem);
   if (update_range_test (rangei, rangej, NULL, 1, opcode, ops, exp,
@@ -2581,7 +2596,16 @@ optimize_range_tests_diff (enum tree_cod
   if (!integer_pow2p (tem1))
 return false;

-  type = unsigned_type_for (type);
+  scalar_int_mode mode = as_a  (TYPE_MODE (type));
+  int prec = GET_MODE_PRECISION (mode);
+  if (TYPE_PRECISION (type) < prec
+  || (wi::to_wide (TYPE_MIN_VALUE (type))
+ != wi::min_value (prec, TYPE_SIGN (type)))
+  || (wi::to_wide (TYPE_MAX_VALUE (type))
+ != wi::max_value (prec, TYPE_SIGN (type
+type = build_nonstandard_integer_type (prec, 1);
+  else
+type = unsigned_type_for (type);
   tem1 = fold_convert (type, tem1);
   tem2 = fold_convert (type, tem2);
   lowi = fold_convert (type, lowi);

Do we want that too, or is the exact type in which we compute these
uninteresting?  Note, unfortunately in this case the enum type has
TYPE_PRECISION 32, just TYPE_MAX_VALUE of 15, and we consider such conversions
useless, so not really sure how can vrp work reliably with that.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #8 from rguenther at suse dot de  ---
On Fri, 30 Nov 2018, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274
> 
> --- Comment #7 from Jakub Jelinek  ---
> The regression started with r265241 BTW.
> 
> Here is a reassoc patch I wrote that also fixes this ICE:
> --- gcc/tree-ssa-reassoc.c.jj   2018-10-23 10:13:25.278875175 +0200
> +++ gcc/tree-ssa-reassoc.c  2018-11-30 11:13:37.232393154 +0100
> @@ -2537,8 +2537,23 @@ optimize_range_tests_xor (enum tree_code
>if (!tree_int_cst_equal (lowxor, highxor))
>  return false;
> 
> +  exp = rangei->exp;
> +  scalar_int_mode mode = as_a  (TYPE_MODE (type));
> +  int prec = GET_MODE_PRECISION (mode);
> +  if (TYPE_PRECISION (type) < prec
> +  || (wi::to_wide (TYPE_MIN_VALUE (type))
> + != wi::min_value (prec, TYPE_SIGN (type)))
> +  || (wi::to_wide (TYPE_MAX_VALUE (type))
> + != wi::max_value (prec, TYPE_SIGN (type
> +{
> +  type = build_nonstandard_integer_type (prec, TYPE_UNSIGNED (type));
> +  exp = fold_convert (type, exp);
> +  lowxor = fold_convert (type, lowxor);
> +  lowi = fold_convert (type, lowi);
> +  highi = fold_convert (type, highi);
> +}
>tem = fold_build1 (BIT_NOT_EXPR, type, lowxor);
> -  exp = fold_build2 (BIT_AND_EXPR, type, rangei->exp, tem);
> +  exp = fold_build2 (BIT_AND_EXPR, type, exp, tem);
>lowj = fold_build2 (BIT_AND_EXPR, type, lowi, tem);
>highj = fold_build2 (BIT_AND_EXPR, type, highi, tem);
>if (update_range_test (rangei, rangej, NULL, 1, opcode, ops, exp,
> @@ -2581,7 +2596,16 @@ optimize_range_tests_diff (enum tree_cod
>if (!integer_pow2p (tem1))
>  return false;
> 
> -  type = unsigned_type_for (type);
> +  scalar_int_mode mode = as_a  (TYPE_MODE (type));
> +  int prec = GET_MODE_PRECISION (mode);
> +  if (TYPE_PRECISION (type) < prec
> +  || (wi::to_wide (TYPE_MIN_VALUE (type))
> + != wi::min_value (prec, TYPE_SIGN (type)))
> +  || (wi::to_wide (TYPE_MAX_VALUE (type))
> + != wi::max_value (prec, TYPE_SIGN (type
> +type = build_nonstandard_integer_type (prec, 1);
> +  else
> +type = unsigned_type_for (type);
>tem1 = fold_convert (type, tem1);
>tem2 = fold_convert (type, tem2);
>lowi = fold_convert (type, lowi);
> 
> Do we want that too, or is the exact type in which we compute these
> uninteresting?  Note, unfortunately in this case the enum type has
> TYPE_PRECISION 32, just TYPE_MAX_VALUE of 15, and we consider such conversions
> useless, so not really sure how can vrp work reliably with that.

I wonder that as well but it has been doing that (to some limited extent)
since forever.  The middle-end really only cares about TYPE_PRECISION
everywhere (but in VRP...).

So I'd happily substitute wi::min/max_value for TYPE_MIN/MAX_VALUE
in vrp_val_min/max ... with the expected "regressions" for
-fstrict-enums (and maybe Ada).

For your patch I think you don't need the mode-precision vs.
type-precision thing or is that simply to make the code not
un-optimal?  But yes, we shouldn't generate new arithmetic
in types with non-matching TYPE_MIN/MAX_VALUE.  Ada for
example does this in the underlying type and uses VCE
exprs to convert between those (IIRC).

The current state really asks for miscompilations (with -fstrict-enums).

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

Richard Biener  changed:

   What|Removed |Added

 CC||ebotcazou at gcc dot gnu.org

--- Comment #9 from Richard Biener  ---
Maybe Eric wants to chime in.  IIRC we went through great lengths to preserve
VRP using TYPE_MIN/MAX_VALUE to elide bounds checking for Ada?

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread ebotcazou at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #10 from Eric Botcazou  ---
> Maybe Eric wants to chime in.  IIRC we went through great lengths to
> preserve VRP using TYPE_MIN/MAX_VALUE to elide bounds checking for Ada?

It's the opposite, we need to hide TYPE_MIN/MAX_VALUE to preserve checks in
Ada, otherwise the optimizer happily removes them (see
ada/gcc-interface/ada-tree.h).

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread ebotcazou at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #11 from Eric Botcazou  ---
> It's the opposite, we need to hide TYPE_MIN/MAX_VALUE to preserve checks in
> Ada, otherwise the optimizer happily removes them.

To be more explicit: in Ada, you can check at run time whether a value is valid
for its nominal subtype (i.e. you can check if it's within the range) so you
cannot have the obvious TYPE_MIN/MAX_VALUE on this nominal subtype, otherwise
these checks will be elided by VRP.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #12 from rguenther at suse dot de  ---
On Fri, 30 Nov 2018, ebotcazou at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274
> 
> --- Comment #11 from Eric Botcazou  ---
> > It's the opposite, we need to hide TYPE_MIN/MAX_VALUE to preserve checks in
> > Ada, otherwise the optimizer happily removes them.
> 
> To be more explicit: in Ada, you can check at run time whether a value is 
> valid
> for its nominal subtype (i.e. you can check if it's within the range) so you
> cannot have the obvious TYPE_MIN/MAX_VALUE on this nominal subtype, otherwise
> these checks will be elided by VRP.

OK, so we don't have coverage for VRP optimizations on such types
from Ada.  That leaves us with -fstrict-enums, defaulted to off.

I seriously question the value of VRP trying to look at
TYPE_MIN/MAX_VALUE then.

[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188

2018-11-30 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88274

--- Comment #13 from Jakub Jelinek  ---
Author: jakub
Date: Fri Nov 30 23:27:23 2018
New Revision: 266701

URL: https://gcc.gnu.org/viewcvs?rev=266701&root=gcc&view=rev
Log:
PR tree-optimization/88274
* tree-ssa-reassoc.c (optimize_range_tests_xor,
optimize_range_tests_diff): If type has smaller precision than
corresponding mode or if it has non-standard min/max, compute
everything in a standard type for the precision.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/tree-ssa-reassoc.c