On Mon, Nov 11, 2013 at 4:04 PM, Kenneth Zadeck <zad...@naturalbridge.com> wrote: > On 11/11/2013 09:42 AM, Richard Biener wrote: >> >> On Mon, Nov 11, 2013 at 3:26 PM, Kenneth Zadeck >> <zad...@naturalbridge.com> wrote: >>> >>> On 11/11/2013 06:49 AM, Richard Biener wrote: >>>> >>>> On Sat, Nov 9, 2013 at 3:23 PM, Kenneth Zadeck >>>> <zad...@naturalbridge.com> >>>> wrote: >>>>> >>>>> On 11/08/2013 05:30 AM, Richard Sandiford wrote: >>>>>> >>>>>> From tree-vrp.c: >>>>>> >>>>>> @@ -1893,6 +1884,10 @@ vrp_int_const_binop (enum tree_code code >>>>>> /* If the singed operation wraps then int_const_binop has done >>>>>> everything we want. */ >>>>>> ; >>>>>> + /* Signed division of -1/0 overflows and by the time it gets here >>>>>> + returns NULL_TREE. */ >>>>>> + else if (!res) >>>>>> + return NULL_TREE; >>>>>> else if ((TREE_OVERFLOW (res) >>>>>> && !TREE_OVERFLOW (val1) >>>>>> && !TREE_OVERFLOW (val2)) >>>>>> >>>>>> Why is this case different from trunk? Or is it a bug-fix independent >>>>>> of wide-int? >>>>> >>>>> the api for division is different for wide-int than it was for >>>>> double-int. >>>> >>>> But this is using a tree API (int_const_binop) that didn't change >>>> (it returned NULL for / 0 previously). So what makes us arrive here >>>> now? (I agree there is a bug in VRP, but it shouldn't manifest itself >>>> only on wide-int) >>>> >>>> Richard. >>> >>> My reading of the code is that is that i changed int_const_binop to >>> return >>> null_tree for this case. >> >> Trunk has: >> >> case TRUNC_DIV_EXPR: >> case FLOOR_DIV_EXPR: case CEIL_DIV_EXPR: >> case EXACT_DIV_EXPR: >> /* This is a shortcut for a common special case. */ >> if (op2.high == 0 && (HOST_WIDE_INT) op2.low > 0 >> && !TREE_OVERFLOW (arg1) >> && !TREE_OVERFLOW (arg2) >> && op1.high == 0 && (HOST_WIDE_INT) op1.low >= 0) >> { >> if (code == CEIL_DIV_EXPR) >> op1.low += op2.low - 1; >> >> res.low = op1.low / op2.low, res.high = 0; >> break; >> } >> >> /* ... fall through ... */ >> >> case ROUND_DIV_EXPR: >> if (op2.is_zero ()) >> return NULL_TREE; >> >> so it already returns NULL_TREE on divide by zero. > > I found the reason!!!! This is one of the many "tree-vrp was not properly > tested for TImode bugs." > > on the trunk, the case 0/(smallest negative number) case will only trigger > overflow in TImode. On the wide-int branch, tree-vrp works at the > precision of the operands so overflow is triggered properly for this case. > So for HImode, the trunk produces the a result for 0/0x80 and then force_fit > code at the bottom of int_const_binop_1 turns this into an overflow tree > value rather than a null tree. > > on the wide-int branch, this case causes the overflow bit to be returned > from the wide-int divide because the overflow case is properly handled for > all types and that overflow is turned into null_tree by the wide-int version > of int_const_binop_1. > > apparently there are no test cases that exercise the true divide by 0 case > but there are test cases that hit the 0/ largest negative number case for > modes smaller than TImode.
You probably mean <largest negative number> / -1? I don't see where NULL_TREE is returned for any overflow case in int_const_binop_1. Ah, you made it so. That looks like a bogus change. What's the reason? int_const_binop_1 is supposed to return a value with TREE_OVERFLOW set in these cases, also required for frontend constant folding. Try compiling const int i = (-__INT_MAX__ - 1) / -1; which should say > ./cc1 -quiet t.c t.c:1:34: warning: integer overflow in expression [-Woverflow] const int i = (-__INT_MAX__ - 1) / -1; ^ but not error or ICE. Seems to work on the branch, but only because the expression is still folded by 12357 /* X / -1 is -X. */ 12358 if (!TYPE_UNSIGNED (type) 12359 && TREE_CODE (arg1) == INTEGER_CST 12360 && wi::eq_p (arg1, -1)) 12361 return fold_convert_loc (loc, type, negate_expr (arg0)); Thus case TRUNC_DIV_EXPR: case EXACT_DIV_EXPR: res = wi::div_trunc (arg1, arg2, sign, &overflow); if (overflow) return NULL_TREE; break; should retain the arg2 == 0 checks (and return NULL_TREE) but otherwise keep overflow handling the same. Richard. > Kenny > >> >>> On the trunk, only rem returns null_tree for divide by 0, on the wide int >>> branch, both div and rem return null tree. >>> >>> I know that this is going to bring on a string of questions that i do not >>> remember the answers to as to why i made that change. but >>> fold-const.c:int_const_binop_1 now returns null_tree and this is just >>> fallout from that change. >>> >>> Kenny >>>>>> >>>>>> Thanks, >>>>>> Richard >>>>> >>>>> >