On Mon, Apr 25, 2016 at 9:21 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> Hello,
>
> a simple transform to replace a more complicated one in fold-const.c.
>
> This patch breaks the testcase gcc.dg/gomp/loop-1.c. Indeed, the C front-end
> folds too eagerly
> newrhs = c_fully_fold (newrhs, false, NULL);
> in build_modify_expr, and by the time the OMP code checks that the increment
> in the for loop has the right form, it sees i=i*2 instead of i=i+i. Since
> the original code is apparently illegal, I guess it isn't that bad... The
> C++ front-end seems fine.
>
> Testcase no-strict-overflow-6.c also breaks. ivcanon is clever enough to
> count how many iterations there are before i*=2 makes i negative, which I
> guess would be great with -fwrapv, but I find it a bit suspicious with just
> -fno-strict-overflow. I adjusted the testcase assuming the ivcanon was doing
> the right thing (with -fstrict-overflow we generate an infinite loop
> instead, so it is still testing that).
>
> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
Ok. Note that I think
/* (X /[ex] A) * A -> X. */
(simplify
(mult (convert? (exact_div @0 @1)) @1)
/* Look through a sign-changing conversion. */
(convert @0))
has a bug as we use operand_equal_p for comparing @1 but
that treats equal but different typed INTEGER_CSTs as equal...
Thus this lacks the tree_nop_conversion_p check.
Thanks,
Richard.
> 2016-04-26 Marc Glisse <marc.gli...@inria.fr>
>
> gcc/
> * genmatch.c (write_predicate): Add ATTRIBUTE_UNUSED.
> * fold-const.c (fold_binary_loc): Remove 2 transformations
> superseded by match.pd.
> * match.pd (x+x -> x*2): Generalize to integers.
>
> gcc/testsuite/
> * gcc.dg/fold-plusmult.c: Adjust.
> * gcc.dg/no-strict-overflow-6.c: Adjust.
> * gcc.dg/gomp/loop-1.c: Xfail some tests.
>
> --
> Marc Glisse
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 235411)
> +++ gcc/fold-const.c (working copy)
> @@ -9949,39 +9949,20 @@ fold_binary_loc (location_t loc,
> /* Transform x * -C into -x * C if x is easily negatable. */
> if (TREE_CODE (op1) == INTEGER_CST
> && tree_int_cst_sgn (op1) == -1
> && negate_expr_p (op0)
> && (tem = negate_expr (op1)) != op1
> && ! TREE_OVERFLOW (tem))
> return fold_build2_loc (loc, MULT_EXPR, type,
> fold_convert_loc (loc, type,
> negate_expr (op0)),
> tem);
>
> - /* (A + A) * C -> A * 2 * C */
> - if (TREE_CODE (arg0) == PLUS_EXPR
> - && TREE_CODE (arg1) == INTEGER_CST
> - && operand_equal_p (TREE_OPERAND (arg0, 0),
> - TREE_OPERAND (arg0, 1), 0))
> - return fold_build2_loc (loc, MULT_EXPR, type,
> - omit_one_operand_loc (loc, type,
> - TREE_OPERAND (arg0, 0),
> - TREE_OPERAND (arg0, 1)),
> - fold_build2_loc (loc, MULT_EXPR, type,
> - build_int_cst (type, 2) ,
> arg1));
> -
> - /* ((T) (X /[ex] C)) * C cancels out if the conversion is
> - sign-changing only. */
> - if (TREE_CODE (arg1) == INTEGER_CST
> - && TREE_CODE (arg0) == EXACT_DIV_EXPR
> - && operand_equal_p (arg1, TREE_OPERAND (arg0, 1), 0))
> - return fold_convert_loc (loc, type, TREE_OPERAND (arg0, 0));
> -
> strict_overflow_p = false;
> if (TREE_CODE (arg1) == INTEGER_CST
> && 0 != (tem = extract_muldiv (op0, arg1, code, NULL_TREE,
> &strict_overflow_p)))
> {
> if (strict_overflow_p)
> fold_overflow_warning (("assuming signed overflow does not "
> "occur when simplifying "
> "multiplication"),
> WARN_STRICT_OVERFLOW_MISC);
> Index: gcc/genmatch.c
> ===================================================================
> --- gcc/genmatch.c (revision 235411)
> +++ gcc/genmatch.c (working copy)
> @@ -3549,21 +3549,21 @@ decision_tree::gen (FILE *f, bool gimple
>
> /* Output code to implement the predicate P from the decision tree DT. */
>
> void
> write_predicate (FILE *f, predicate_id *p, decision_tree &dt, bool gimple)
> {
> fprintf (f, "\nbool\n"
> "%s%s (tree t%s%s)\n"
> "{\n", gimple ? "gimple_" : "tree_", p->id,
> p->nargs > 0 ? ", tree *res_ops" : "",
> - gimple ? ", tree (*valueize)(tree)" : "");
> + gimple ? ", tree (*valueize)(tree) ATTRIBUTE_UNUSED" : "");
> /* Conveniently make 'type' available. */
> fprintf_indent (f, 2, "tree type = TREE_TYPE (t);\n");
>
> if (!gimple)
> fprintf_indent (f, 2, "if (TREE_SIDE_EFFECTS (t)) return false;\n");
> dt.root->gen_kids (f, 2, gimple);
>
> fprintf_indent (f, 2, "return false;\n"
> "}\n");
> }
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd (revision 235411)
> +++ gcc/match.pd (working copy)
> @@ -1621,25 +1621,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Canonicalization of binary operations. */
>
> /* Convert X + -C into X - C. */
> (simplify
> (plus @0 REAL_CST@1)
> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
> (with { tree tem = const_unop (NEGATE_EXPR, type, @1); }
> (if (!TREE_OVERFLOW (tem) || !flag_trapping_math)
> (minus @0 { tem; })))))
>
> -/* Convert x+x into x*2.0. */
> +/* Convert x+x into x*2. */
> (simplify
> (plus @0 @0)
> (if (SCALAR_FLOAT_TYPE_P (type))
> - (mult @0 { build_real (type, dconst2); })))
> + (mult @0 { build_real (type, dconst2); })
> + (if (INTEGRAL_TYPE_P (type))
> + (mult @0 { build_int_cst (type, 2); }))))
>
> (simplify
> (minus integer_zerop @1)
> (negate @1))
>
> /* (ARG0 - ARG1) is the same as (-ARG1 + ARG0). So check whether
> ARG0 is zero and X + ARG0 reduces to X, since that would mean
> (-ARG1 + ARG0) reduces to -ARG1. */
> (simplify
> (minus real_zerop@0 @1)
> Index: gcc/testsuite/gcc.dg/fold-plusmult.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/fold-plusmult.c (revision 235411)
> +++ gcc/testsuite/gcc.dg/fold-plusmult.c (working copy)
> @@ -4,11 +4,11 @@
> int test1 (int a)
> {
> return 2*a + 2*a;
> }
>
> int test2 (int a)
> {
> return (a + a)*2;
> }
>
> -/* { dg-final { scan-tree-dump-times "<a> \\\* 4" 2 "original" } } */
> +/* { dg-final { scan-tree-dump-times "a \\\* 4" 2 "original" } } */
> Index: gcc/testsuite/gcc.dg/gomp/loop-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/gomp/loop-1.c (revision 235411)
> +++ gcc/testsuite/gcc.dg/gomp/loop-1.c (working copy)
> @@ -37,28 +37,28 @@ f1 (int x)
> ;
> #pragma omp for
> for (i = 5; bar (i) > i; i++) /* { dg-error "condition expression refers
> to iteration variable" } */
> ;
> #pragma omp for
> for (i = 5; i <= baz (&i); i++) /* { dg-error "condition expression
> refers to iteration variable" } */
> ;
> #pragma omp for
> for (i = 5; i <= i; i++) /* { dg-error "invalid controlling
> predicate|condition expression refers to iteration variable" } */
> ;
> - #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" } */
> - for (i = 5; i < 16; i += i)
> + #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" "" { xfail *-*-* } } */
> + for (i = 5; i < 16; i += i) /* { dg-bogus "invalid increment expression"
> "" { xfail *-*-* } } */
> ;
> #pragma omp for
> for (i = 5; i < 16; i = i + 2 * i) /* { dg-error "invalid increment
> expression|increment expression refers to iteration variable" } */
> ;
> - #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" } */
> - for (i = 5; i < 16; i = i + i)
> + #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" "" { xfail *-*-* } } */
> + for (i = 5; i < 16; i = i + i) /* { dg-bogus "invalid increment
> expression" "" { xfail *-*-* } } */
> ;
> #pragma omp for
> for (i = 5; i < 16; i = i + bar (i)) /* { dg-error "increment expression
> refers to iteration variable" } */
> ;
> #pragma omp for
> for (i = 5; i < 16; i = baz (&i) + i) /* { dg-error "increment expression
> refers to iteration variable" } */
> ;
> #pragma omp for
> for (i = 5; i < 16; i += bar (i)) /* { dg-error "increment expression
> refers to iteration variable" } */
> ;
> @@ -174,28 +174,28 @@ f2 (int x)
> ;
> #pragma omp for
> for (int i = 5; bar (i) > i; i++) /* { dg-error "condition expression
> refers to iteration variable" } */
> ;
> #pragma omp for
> for (int i = 5; i <= baz (&i); i++) /* { dg-error "condition expression
> refers to iteration variable" } */
> ;
> #pragma omp for
> for (int i = 5; i <= i; i++) /* { dg-error "invalid controlling
> predicate|condition expression refers to iteration variable" } */
> ;
> - #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" } */
> - for (int i = 5; i < 16; i += i)
> + #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" "" { xfail *-*-* } } */
> + for (int i = 5; i < 16; i += i) /* { dg-bogus "invalid increment
> expression" "" { xfail *-*-* } } */
> ;
> #pragma omp for
> for (int i = 5; i < 16; i = i + 2 * i) /* { dg-error "invalid increment
> expression|increment expression refers to iteration variable" } */
> ;
> - #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" } */
> - for (int i = 5; i < 16; i = i + i)
> + #pragma omp for /* { dg-error "increment expression refers to iteration
> variable" "" { xfail *-*-* } } */
> + for (int i = 5; i < 16; i = i + i) /* { dg-bogus "invalid increment
> expression" "" { xfail *-*-* } } */
> ;
> #pragma omp for
> for (int i = 5; i < 16; i = i + bar (i)) /* { dg-error "increment
> expression refers to iteration variable" } */
> ;
> #pragma omp for
> for (int i = 5; i < 16; i = baz (&i) + i) /* { dg-error "increment
> expression refers to iteration variable" } */
> ;
> #pragma omp for
> for (int i = 5; i < 16; i += bar (i)) /* { dg-error "increment expression
> refers to iteration variable" } */
> ;
> Index: gcc/testsuite/gcc.dg/no-strict-overflow-6.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/no-strict-overflow-6.c (revision 235411)
> +++ gcc/testsuite/gcc.dg/no-strict-overflow-6.c (working copy)
> @@ -7,14 +7,14 @@
> strict overflow semantics. We don't test this with
> -fstrict-overflow because it turns into an infinite loop. That is
> OK but it would also be OK to not do that. */
>
> int
> foo ()
> {
> int i, bits;
> for (i = 1, bits = 1; i > 0; i += i)
> ++bits;
> - return bits;
> + return bits - sizeof(int) * __CHAR_BIT__;
> }
>
> -/* { dg-final { scan-tree-dump "return bits" "optimized" } } */
> +/* { dg-final { scan-tree-dump "return 0" "optimized" } } */
>
