On Tue, 2 Mar 2021, Jakub Jelinek wrote:
> Hi!
>
> The Wstringop-overflow-25.c testcase doesn't emit one of the expected
> warnings on targets that don't do short curcuiting due to target costs
> (or e.g. with --param=logical-op-non-short-circuit=0 on all targets).
>
> The problem is that only reassoc2 optimizes:
> _49 ={v} unsigned_value_source;
> if (_49 == 0)
> goto <bb 7>; [50.00%]
> else
> goto <bb 5>; [50.00%]
>
> <bb 5> [local count: 536870913]:
> if (_49 > 2)
> goto <bb 7>; [50.00%]
> else
> goto <bb 6>; [50.00%]
>
> <bb 6> [local count: 268435457]:
> _53 = _49 + 1;
> into:
> _49 ={v} unsigned_value_source;
> _48 = _49 + 18446744073709551615;
> _1 = _48 > 1;
> if (_1 != 0)
> goto <bb 6>; [50.00%]
> else
> goto <bb 5>; [50.00%]
>
> <bb 5> [local count: 268435457]:
> _53 = _49 + 1;
> (but, note the _1 = _48 > 1; if (_1 != 0)),
> then dom3 is run and because of that if (_1 != 0) vs. if (_48 > 1) doesn't
> register edge asserts for _48 and _49) and so we don't get
> SSA_NAME_RANGE_INFO for _53 (and ditto for vrp2) and only afterwards comes
> forwprop4 that canonicalizes it to if (_48 > 1). While with
> --param=logical-op-non-short-circuit=1 it is already reassoc1 that optimizes
> it and forwprop3 that propagates it, so we have on the SSA_NAME
> corresponding to _53 above SSA_NAME_RANGE_INFO and during expansion we warn.
>
> The following patch fixes it by handling those not yet propagated
> comparisons into GIMPLE_COND in register_edge_assert_for. We already
> have all the infrastructure there to handle the
> --param=logical-op-non-short-circuit=1
> | and &s.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2021-03-02 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/95757
> * tree-vrp.c (register_edge_assert_for): Remove superfluous ()s around
> condition. Call register_edge_assert_for_1 for == 0, != 0, == 1 and
> != 1 comparisons if name is lhs of a comparison.
>
> --- gcc/tree-vrp.c.jj 2021-02-25 10:16:46.495355574 +0100
> +++ gcc/tree-vrp.c 2021-03-01 18:50:48.586354899 +0100
> @@ -2180,8 +2180,8 @@ register_edge_assert_for (tree name, edg
> /* In the case of NAME == 1 or NAME != 0, for BIT_AND_EXPR defining
> statement of NAME we can assert both operands of the BIT_AND_EXPR
> have nonzero value. */
> - if (((comp_code == EQ_EXPR && integer_onep (val))
> - || (comp_code == NE_EXPR && integer_zerop (val))))
> + if ((comp_code == EQ_EXPR && integer_onep (val))
> + || (comp_code == NE_EXPR && integer_zerop (val)))
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (name);
>
> @@ -2193,28 +2193,36 @@ register_edge_assert_for (tree name, edg
> register_edge_assert_for_1 (op0, NE_EXPR, e, asserts);
> register_edge_assert_for_1 (op1, NE_EXPR, e, asserts);
> }
> + else if (is_gimple_assign (def_stmt)
> + && (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
> + == tcc_comparison))
> + register_edge_assert_for_1 (name, NE_EXPR, e, asserts);
> }
>
> /* In the case of NAME == 0 or NAME != 1, for BIT_IOR_EXPR defining
> statement of NAME we can assert both operands of the BIT_IOR_EXPR
> have zero value. */
> - if (((comp_code == EQ_EXPR && integer_zerop (val))
> - || (comp_code == NE_EXPR && integer_onep (val))))
> + if ((comp_code == EQ_EXPR && integer_zerop (val))
> + || (comp_code == NE_EXPR
> + && integer_onep (val)
> + && TYPE_PRECISION (TREE_TYPE (name)) == 1))
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (name);
>
> /* For BIT_IOR_EXPR only if NAME == 0 both operands have
> necessarily zero value, or if type-precision is one. */
> if (is_gimple_assign (def_stmt)
> - && (gimple_assign_rhs_code (def_stmt) == BIT_IOR_EXPR
> - && (TYPE_PRECISION (TREE_TYPE (name)) == 1
> - || comp_code == EQ_EXPR)))
> + && gimple_assign_rhs_code (def_stmt) == BIT_IOR_EXPR)
> {
> tree op0 = gimple_assign_rhs1 (def_stmt);
> tree op1 = gimple_assign_rhs2 (def_stmt);
> register_edge_assert_for_1 (op0, EQ_EXPR, e, asserts);
> register_edge_assert_for_1 (op1, EQ_EXPR, e, asserts);
> }
> + else if (is_gimple_assign (def_stmt)
> + && (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
> + == tcc_comparison))
> + register_edge_assert_for_1 (name, EQ_EXPR, e, asserts);
> }
>
> /* Sometimes we can infer ranges from (NAME & MASK) == VALUE. */
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
