This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][20/n] Remove GENERIC stmt combining from SCCVN
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 14 Sep 2015 11:41:25 +0200
- Subject: Re: [PATCH][20/n] Remove GENERIC stmt combining from SCCVN
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1507241400450 dot 19642 at zhemvz dot fhfr dot qr> <alpine dot LSU dot 2 dot 11 dot 1509141036321 dot 13444 at zhemvz dot fhfr dot qr> <alpine dot LSU dot 2 dot 11 dot 1509141052400 dot 13444 at zhemvz dot fhfr dot qr>
> 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 old code was propagating the comparison inside the arms of COND_EXPR
(fold_binary_op_with_conditional_arg) before applying the transformation:
if ((short_short_integer) x == 127 ? .gnat_rcheck_CE_Overflow_Check
("overflow_sum3.adb", 14);, 1 : 1)
The new code does the reverse, but the old behavior can be easily restored:
Index: fold-const.c
===================================================================
--- fold-const.c (revision 227729)
+++ fold-const.c (working copy)
@@ -9025,10 +9025,6 @@ fold_binary_loc (location_t loc,
&& tree_swap_operands_p (arg0, arg1, true))
return fold_build2_loc (loc, swap_tree_comparison (code), type, op1,
op0);
- tem = generic_simplify (loc, code, type, op0, op1);
- if (tem)
- return tem;
-
/* ARG0 is the first operand of EXPR, and ARG1 is the second operand.
First check for cases where an arithmetic operation is applied to a
@@ -9114,6 +9110,10 @@ fold_binary_loc (location_t loc,
}
}
+ tem = generic_simplify (loc, code, type, op0, op1);
+ if (tem)
+ return tem;
+
switch (code)
{
case MEM_REF:
is sufficient to fix the regression.
> 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 }
That's old code and the comment makes it quite clear why this is done though.
--
Eric Botcazou