| Message ID | alpine.LSU.2.11.1507241400450.19642@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
> * fold-const.c (fold_binary_loc): Move simplifying of comparisons > against the highest or lowest possible integer ... > * match.pd: ... as patterns here. This incorrectly dropped the calls to omit_one_operand_loc, resulting in the failure of the attached Ada test: if the operand has side effects, you cannot replace the entire comparison with just 'true' or 'false'. * gnat.dg/overflow_sum3.adb: New test.
On Sat, 12 Sep 2015, Eric Botcazou wrote: > > * fold-const.c (fold_binary_loc): Move simplifying of comparisons > > against the highest or lowest possible integer ... > > * match.pd: ... as patterns here. > > This incorrectly dropped the calls to omit_one_operand_loc, resulting in the > failure of the attached Ada test: if the operand has side effects, you cannot > replace the entire comparison with just 'true' or 'false'. Still trying to reproduce, but I suppose you hit /* Comparisons with the highest or lowest possible integer of the specified precision will have known values. */ (simplify (cmp (convert?@2 @0) INTEGER_CST@1) (if ((INTEGRAL_TYPE_P (TREE_TYPE (@1)) || POINTER_TYPE_P (TREE_TYPE (@1))) && tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))) (with { tree arg1_type = TREE_TYPE (@1); unsigned int prec = TYPE_PRECISION (arg1_type); wide_int max = wi::max_value (arg1_type); wide_int signed_max = wi::max_value (prec, SIGNED); wide_int min = wi::min_value (arg1_type); } (switch (if (wi::eq_p (@1, max)) (switch (if (cmp == GT_EXPR) { constant_boolean_node (false, type); }) (if (cmp == GE_EXPR) (eq @2 @1)) (if (cmp == LE_EXPR) { constant_boolean_node (true, type); }) this which should handle side-effects in @0 just fine: /* #line 2019 "/space/rguenther/src/svn/trunk/gcc/match.pd" */ if (cmp == LE_EXPR) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Applying pattern match.pd:2020, %s:%d\n", __FILE__, __LINE__); tree res; res = constant_boolean_node (true, type); if (TREE_SIDE_EFFECTS (captures[0])) res = build2_loc (loc, COMPOUND_EXPR, type, fold_ignored_result (captures[0]), res); return res; note that genmatch "inlines" omit_one_operand, so you only see fold_ignored_result here. So maybe the issue is with some other pattern or was latent elsewehere. I'll have a closer look once I manage to reproduce the issue. Richard. > > * gnat.dg/overflow_sum3.adb: New test. > >
On Mon, 14 Sep 2015, Richard Biener wrote: > On Sat, 12 Sep 2015, Eric Botcazou wrote: > > > > * fold-const.c (fold_binary_loc): Move simplifying of comparisons > > > against the highest or lowest possible integer ... > > > * match.pd: ... as patterns here. > > > > This incorrectly dropped the calls to omit_one_operand_loc, resulting in the > > failure of the attached Ada test: if the operand has side effects, you cannot > > replace the entire comparison with just 'true' or 'false'. > > Still trying to reproduce, but I suppose you hit > > /* Comparisons with the highest or lowest possible integer of > the specified precision will have known values. */ > (simplify > (cmp (convert?@2 @0) INTEGER_CST@1) > (if ((INTEGRAL_TYPE_P (TREE_TYPE (@1)) || POINTER_TYPE_P (TREE_TYPE > (@1))) > && tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))) > (with > { > tree arg1_type = TREE_TYPE (@1); > unsigned int prec = TYPE_PRECISION (arg1_type); > wide_int max = wi::max_value (arg1_type); > wide_int signed_max = wi::max_value (prec, SIGNED); > wide_int min = wi::min_value (arg1_type); > } > (switch > (if (wi::eq_p (@1, max)) > (switch > (if (cmp == GT_EXPR) > { constant_boolean_node (false, type); }) > (if (cmp == GE_EXPR) > (eq @2 @1)) > (if (cmp == LE_EXPR) > { constant_boolean_node (true, type); }) > > this which should handle side-effects in @0 just fine: > > /* #line 2019 "/space/rguenther/src/svn/trunk/gcc/match.pd" */ > if (cmp == LE_EXPR) > { > if (dump_file && (dump_flags & TDF_DETAILS)) > fprintf (dump_file, "Applying pattern match.pd:2020, %s:%d\n", __FILE__, > __LINE__); > tree res; > res = constant_boolean_node (true, type); > if (TREE_SIDE_EFFECTS (captures[0])) > res = build2_loc (loc, COMPOUND_EXPR, type, > fold_ignored_result (captures[0]), res); > return res; > > note that genmatch "inlines" omit_one_operand, so you only see > fold_ignored_result here. > > So maybe the issue is with some other pattern or was latent > elsewehere. I'll have a closer look once I manage to reproduce > the issue. Ok, so it's folding x == 127 ? .gnat_rcheck_CE_Overflow_Check ("overflow_sum3.adb", 14);, 0 : (short_short_integer) x + 1 <= 127 where op0 (the COND_EXPR) does not have TREE_SIDE_EFFECTS set but its operand 1 has: (gdb) p debug_tree (op0) <cond_expr 0x7ffff68cbf90 type <integer_type 0x7ffff6572dc8 short_short_integer sizes-gimplified public visited QI size <integer_cst 0x7ffff68ccca8 constant visited 8> unit size <integer_cst 0x7ffff68cccc0 constant visited 1> align 8 symtab 0 alias set -1 canonical type 0x7ffff6572dc8 precision 8 min <integer_cst 0x7ffff656a678 -128> max <integer_cst 0x7ffff656a6c0 127> context <translation_unit_decl 0x7ffff7ff81e0 D.24> RM size <integer_cst 0x7ffff68ccca8 8> chain <type_decl 0x7ffff6900b48 short_short_integer>> arg 0 <eq_expr 0x7ffff6573938 ... arg 1 <compound_expr 0x7ffff65739b0 type <integer_type 0x7ffff6572dc8 short_short_integer> side-effects ... arg 2 <plus_expr 0x7ffff6573910 type <integer_type 0x7ffff6572dc8 short_short_integer> ... that's unexpected to the code generated by genmatch and I don't see how omit_one_operand would handle that either. The COND_EXPR is originally built with TREE_SIDE_EFFECTS set but: Hardware watchpoint 7: *$43 Old value = 65595 New value = 59 emit_check (gnu_cond=<eq_expr 0x7ffff6573938>, gnu_expr=<plus_expr 0x7ffff6573910>, reason=10, gnat_node=2320) at /space/rguenther/src/svn/trunk/gcc/ada/gcc-interface/trans.c:8823 8823 return gnu_result; $45 = 0 so the Ada frontend resets the flag (improperly?): emit_check (gnu_cond=<eq_expr 0x7ffff6573938>, gnu_expr=<plus_expr 0x7ffff6573910>, reason=10, gnat_node=2320) at /space/rguenther/src/svn/trunk/gcc/ada/gcc-interface/trans.c:8823 8823 return gnu_result; $45 = 0 (gdb) l 8818 8819 /* GNU_RESULT has side effects if and only if GNU_EXPR has: 8820 we don't need to evaluate it just for the check. */ 8821 TREE_SIDE_EFFECTS (gnu_result) = TREE_SIDE_EFFECTS (gnu_expr); 8822 8823 return gnu_result; 8824 } Richard.
> Still trying to reproduce, but I suppose you hit The testcase fails as of r227729 on x86-64/Linux. > /* Comparisons with the highest or lowest possible integer of > the specified precision will have known values. */ > (simplify > (cmp (convert?@2 @0) INTEGER_CST@1) > (if ((INTEGRAL_TYPE_P (TREE_TYPE (@1)) || POINTER_TYPE_P (TREE_TYPE > (@1))) > && tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))) > (with > { > tree arg1_type = TREE_TYPE (@1); > unsigned int prec = TYPE_PRECISION (arg1_type); > wide_int max = wi::max_value (arg1_type); > wide_int signed_max = wi::max_value (prec, SIGNED); > wide_int min = wi::min_value (arg1_type); > } > (switch > (if (wi::eq_p (@1, max)) > (switch > (if (cmp == GT_EXPR) > { constant_boolean_node (false, type); }) > (if (cmp == GE_EXPR) > (eq @2 @1)) > (if (cmp == LE_EXPR) > { constant_boolean_node (true, type); }) > > this which should handle side-effects in @0 just fine: > > /* #line 2019 "/space/rguenther/src/svn/trunk/gcc/match.pd" */ > if (cmp == LE_EXPR) > { > if (dump_file && (dump_flags & TDF_DETAILS)) > fprintf (dump_file, "Applying pattern match.pd:2020, %s:%d\n", __FILE__, > __LINE__); > tree res; > res = constant_boolean_node (true, type); > if (TREE_SIDE_EFFECTS (captures[0])) > res = build2_loc (loc, COMPOUND_EXPR, type, > fold_ignored_result (captures[0]), res); > return res; > > note that genmatch "inlines" omit_one_operand, so you only see > fold_ignored_result here. I see, then for some reason TREE_SIDE_EFFECTS is not set here.
Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 226140) +++ gcc/fold-const.c (working copy) @@ -11651,123 +11463,6 @@ fold_binary_loc (location_t loc, } } - /* Comparisons with the highest or lowest possible integer of - the specified precision will have known values. */ - { - tree arg1_type = TREE_TYPE (arg1); - unsigned int prec = TYPE_PRECISION (arg1_type); - - if (TREE_CODE (arg1) == INTEGER_CST - && (INTEGRAL_TYPE_P (arg1_type) || POINTER_TYPE_P (arg1_type))) - { - wide_int max = wi::max_value (arg1_type); - wide_int signed_max = wi::max_value (prec, SIGNED); - wide_int min = wi::min_value (arg1_type); - - if (wi::eq_p (arg1, max)) - switch (code) - { - case GT_EXPR: - return omit_one_operand_loc (loc, type, integer_zero_node, arg0); - - case GE_EXPR: - return fold_build2_loc (loc, EQ_EXPR, type, op0, op1); - - case LE_EXPR: - return omit_one_operand_loc (loc, type, integer_one_node, arg0); - - case LT_EXPR: - return fold_build2_loc (loc, NE_EXPR, type, op0, op1); - - /* The GE_EXPR and LT_EXPR cases above are not normally - reached because of previous transformations. */ - - default: - break; - } - else if (wi::eq_p (arg1, max - 1)) - switch (code) - { - case GT_EXPR: - arg1 = const_binop (PLUS_EXPR, arg1, - build_int_cst (TREE_TYPE (arg1), 1)); - return fold_build2_loc (loc, EQ_EXPR, type, - fold_convert_loc (loc, - TREE_TYPE (arg1), arg0), - arg1); - case LE_EXPR: - arg1 = const_binop (PLUS_EXPR, arg1, - build_int_cst (TREE_TYPE (arg1), 1)); - return fold_build2_loc (loc, NE_EXPR, type, - fold_convert_loc (loc, TREE_TYPE (arg1), - arg0), - arg1); - default: - break; - } - else if (wi::eq_p (arg1, min)) - switch (code) - { - case LT_EXPR: - return omit_one_operand_loc (loc, type, integer_zero_node, arg0); - - case LE_EXPR: - return fold_build2_loc (loc, EQ_EXPR, type, op0, op1); - - case GE_EXPR: - return omit_one_operand_loc (loc, type, integer_one_node, arg0); - - case GT_EXPR: - return fold_build2_loc (loc, NE_EXPR, type, op0, op1); - - default: - break; - } - else if (wi::eq_p (arg1, min + 1)) - switch (code) - { - case GE_EXPR: - arg1 = const_binop (MINUS_EXPR, arg1, - build_int_cst (TREE_TYPE (arg1), 1)); - return fold_build2_loc (loc, NE_EXPR, type, - fold_convert_loc (loc, - TREE_TYPE (arg1), arg0), - arg1); - case LT_EXPR: - arg1 = const_binop (MINUS_EXPR, arg1, - build_int_cst (TREE_TYPE (arg1), 1)); - return fold_build2_loc (loc, EQ_EXPR, type, - fold_convert_loc (loc, TREE_TYPE (arg1), - arg0), - arg1); - default: - break; - } - - else if (wi::eq_p (arg1, signed_max) - && TYPE_UNSIGNED (arg1_type) - /* We will flip the signedness of the comparison operator - associated with the mode of arg1, so the sign bit is - specified by this mode. Check that arg1 is the signed - max associated with this sign bit. */ - && prec == GET_MODE_PRECISION (TYPE_MODE (arg1_type)) - /* signed_type does not work on pointer types. */ - && INTEGRAL_TYPE_P (arg1_type)) - { - /* The following case also applies to X < signed_max+1 - and X >= signed_max+1 because previous transformations. */ - if (code == LE_EXPR || code == GT_EXPR) - { - tree st = signed_type_for (arg1_type); - return fold_build2_loc (loc, - code == LE_EXPR ? GE_EXPR : LT_EXPR, - type, fold_convert_loc (loc, st, arg0), - build_int_cst (st, 0)); - } - } - } - } - /* If we are comparing an ABS_EXPR with a constant, we can convert all the cases into explicit comparisons, but they may well not be faster than doing the ABS and one comparison. Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 226140) +++ gcc/match.pd (working copy) @@ -1807,6 +1864,73 @@ (define_operator_list CBRT BUILT_IN_CBRT { constant_boolean_node (cmp == NE_EXPR, type); }))) +/* Non-equality compare simplifications from fold_binary */ +(for cmp (lt gt le ge) + /* Comparisons with the highest or lowest possible integer of + the specified precision will have known values. */ + (simplify + (cmp (convert?@2 @0) INTEGER_CST@1) + (if ((INTEGRAL_TYPE_P (TREE_TYPE (@1)) || POINTER_TYPE_P (TREE_TYPE (@1))) + && tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))) + (with + { + tree arg1_type = TREE_TYPE (@1); + unsigned int prec = TYPE_PRECISION (arg1_type); + wide_int max = wi::max_value (arg1_type); + wide_int signed_max = wi::max_value (prec, SIGNED); + wide_int min = wi::min_value (arg1_type); + } + (switch + (if (wi::eq_p (@1, max)) + (switch + (if (cmp == GT_EXPR) + { constant_boolean_node (false, type); }) + (if (cmp == GE_EXPR) + (eq @2 @1)) + (if (cmp == LE_EXPR) + { constant_boolean_node (true, type); }) + (if (cmp == LT_EXPR) + (ne @2 @1)))) + (if (wi::eq_p (@1, max - 1)) + (switch + (if (cmp == GT_EXPR) + (eq @2 { wide_int_to_tree (TREE_TYPE (@1), wi::add (@1, 1)); })) + (if (cmp == LE_EXPR) + (ne @2 { wide_int_to_tree (TREE_TYPE (@1), wi::add (@1, 1)); })))) + (if (wi::eq_p (@1, min)) + (switch + (if (cmp == LT_EXPR) + { constant_boolean_node (false, type); }) + (if (cmp == LE_EXPR) + (eq @2 @1)) + (if (cmp == GE_EXPR) + { constant_boolean_node (true, type); }) + (if (cmp == GT_EXPR) + (ne @2 @1)))) + (if (wi::eq_p (@1, min + 1)) + (switch + (if (cmp == GE_EXPR) + (ne @2 { wide_int_to_tree (TREE_TYPE (@1), wi::sub (@1, 1)); })) + (if (cmp == LT_EXPR) + (eq @2 { wide_int_to_tree (TREE_TYPE (@1), wi::sub (@1, 1)); })))) + (if (wi::eq_p (@1, signed_max) + && TYPE_UNSIGNED (arg1_type) + /* We will flip the signedness of the comparison operator + associated with the mode of @1, so the sign bit is + specified by this mode. Check that @1 is the signed + max associated with this sign bit. */ + && prec == GET_MODE_PRECISION (TYPE_MODE (arg1_type)) + /* signed_type does not work on pointer types. */ + && INTEGRAL_TYPE_P (arg1_type)) + /* The following case also applies to X < signed_max+1 + and X >= signed_max+1 because previous transformations. */ + (if (cmp == LE_EXPR || cmp == GT_EXPR) + (with { tree st = signed_type_for (arg1_type); } + (if (cmp == LE_EXPR) + (ge (convert:st @0) { build_zero_cst (st); }) + (lt (convert:st @0) { build_zero_cst (st); })))))))))) + + /* bool_var != 0 becomes bool_var. */ (simplify (ne @0 integer_zerop@1)