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.

Reply via email to