On Tue, Sep 24, 2024 at 2:59 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> For generic, `a != 0 ? a * b : 0` would match where `b` would be an expression
> which trap (in the case of the testcase, it was an integer division but it
> could be any).
>
> This adds a new helper function, expr_no_side_effects_p which tests if there
> is no side effects
> and the expression is not trapping which might be used in other locations.
>
> Changes since v1:
> * v2: Add move check to helper function instead of inlining it.
>
> PR middle-end/116772
OK.
Thanks,
Richard.
> gcc/ChangeLog:
>
> * generic-match-head.cc (expr_no_side_effects_p): New function
> * gimple-match-head.cc (expr_no_side_effects_p): New function
> * match.pd (`a != 0 ? a / b : 0`): Check expr_no_side_effects_p.
> (`a != 0 ? a * b : 0`, `a != 0 ? a & b : 0`): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr116772-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/generic-match-head.cc | 12 ++++++++++++
> gcc/gimple-match-head.cc | 10 ++++++++++
> gcc/match.pd | 10 ++++++++--
> gcc/testsuite/gcc.dg/torture/pr116772-1.c | 24 +++++++++++++++++++++++
> 4 files changed, 54 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr116772-1.c
>
> diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> index 641d8e9b2de..42dee626613 100644
> --- a/gcc/generic-match-head.cc
> +++ b/gcc/generic-match-head.cc
> @@ -115,6 +115,18 @@ optimize_successive_divisions_p (tree, tree)
> return false;
> }
>
> +/* Returns true if the expression T has no side effects
> + including not trapping. */
> +static inline bool
> +expr_no_side_effects_p (tree t)
> +{
> + if (TREE_SIDE_EFFECTS (t))
> + return false;
> + if (generic_expr_could_trap_p (t))
> + return false;
> + return true;
> +}
> +
> /* Return true if EXPR1 and EXPR2 have the same value, but not necessarily
> same type. The types can differ through nop conversions. */
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index b5d4a71ddc5..4147a0eb38a 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -145,6 +145,16 @@ optimize_vectors_before_lowering_p ()
> return !cfun || (cfun->curr_properties & PROP_gimple_lvec) == 0;
> }
>
> +/* Returns true if the expression T has no side effects
> + including not trapping. */
> +static inline bool
> +expr_no_side_effects_p (tree t)
> +{
> + /* For gimple, there should only be gimple val's here. */
> + gcc_assert (is_gimple_val (t));
> + return true;
> +}
> +
> /* Return true if pow(cst, x) should be optimized into exp(log(cst) * x).
> As a workaround for SPEC CPU2017 628.pop2_s, don't do it if arg0
> is an exact integer, arg1 = phi_res +/- cst1 and phi_res = PHI <cst2, ...>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 940292d0d49..1b88168532d 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4679,7 +4679,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (simplify
> (cond (ne @0 integer_zerop) (op@2 @3 @1) integer_zerop )
> (if (bitwise_equal_p (@0, @3)
> - && tree_expr_nonzero_p (@1))
> + && tree_expr_nonzero_p (@1)
> + /* Cannot make a expression with side effects
> + unconditional. */
> + && expr_no_side_effects_p (@3))
> @2)))
>
> /* Note we prefer the != case here
> @@ -4689,7 +4692,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (for op (mult bit_and)
> (simplify
> (cond (ne @0 integer_zerop) (op:c@2 @1 @3) integer_zerop)
> - (if (bitwise_equal_p (@0, @3))
> + (if (bitwise_equal_p (@0, @3)
> + /* Cannot make a expression with side effects
> + unconditional. */
> + && expr_no_side_effects_p (@1))
> @2)))
>
> /* Simplifications of shift and rotates. */
> diff --git a/gcc/testsuite/gcc.dg/torture/pr116772-1.c
> b/gcc/testsuite/gcc.dg/torture/pr116772-1.c
> new file mode 100644
> index 00000000000..eedd0398af1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr116772-1.c
> @@ -0,0 +1,24 @@
> +/* { dg-do run } */
> +/* PR middle-end/116772 */
> +/* The division by `/b` should not
> + be made uncondtional. */
> +
> +int mult0(int a,int b) __attribute__((noipa));
> +
> +int mult0(int a,int b){
> + return (b!=0 ? (a/b)*b : 0);
> +}
> +
> +int bit_and0(int a,int b) __attribute__((noipa));
> +
> +int bit_and0(int a,int b){
> + return (b!=0 ? (a/b)&b : 0);
> +}
> +
> +int main() {
> + if (mult0(3, 0) != 0)
> + __builtin_abort();
> + if (bit_and0(3, 0) != 0)
> + __builtin_abort();
> + return 0;
> +}
> --
> 2.43.0
>
