[Bug tree-optimization/88274] ICE in check, at tree-vrp.c:188
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
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
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
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
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
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
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
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
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
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
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
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
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