Hi Richard,
Thanks for the review.
On 29 April 2016 at 20:47, Richard Biener <richard.guent...@gmail.com> wrote:
> On Sun, Apr 17, 2016 at 1:14 AM, kugan
> <kugan.vivekanandara...@linaro.org> wrote:
>> As explained in PR61839,
>>
>> Following difference results in extra instructions:
>> - c = b != 0 ? 486097858 : 972195717;
>> + c = a + 972195718 >> (b != 0);
>>
>> As suggested in PR, attached patch converts CST BINOP COND_EXPR to COND_EXPR
>> ? (CST BINOP 1) : (CST BINOP 0).
>>
>> Bootstrapped and regression tested for x86-64-linux-gnu with no new
>> regression. Is this OK for statege-1.
>
> You are missing a testcase.
>
> I think the transform can be generalized to any two-value value-range by
> instead of
>
> lhs = cond_res ? (cst binop 1) : (cst binop 0)
>
> emitting
>
> lhs = tmp == val1 ? (cst binop val1) : (cst binop val2);
>
> In the PR I asked the transform to be only carried out if cond_res and
> tmp have a single use (and thus they'd eventually vanish).
>
> I'm not sure if a general two-value "constant" propagation is profitable
> which is why I was originally asking for the pattern to only apply
> if the resulting value is used in a comparison which we could then
> in turn simplify by substituting COND_RES (or ! COND_RES) for it.
> For the general two-value case we'd substitute it with tmp [=!]= val[12]
> dependent on which constant is cheaper to test for.
>
> So I think this needs some exploring work on which way to go
> and which transform is profitable in the end. I think the general
> two-value case feeding a condition will be always profitable.
Please find a modified version which checks for two-valued variable
and uses this to optimize. In the attached test case (in function
bar), we end up doing the conversion twice.
Bootstrapped and regression tested on x86_64-linux-gnu without no new
regressions. Is this OK for trunk?
Thanks,
Kugan
gcc/testsuite/ChangeLog:
2016-08-09 Kugan Vivekanandarajah <kug...@linaro.org>
PR tree-optimization/61839
* gcc.dg/tree-ssa/pr61839.c: New test.
gcc/ChangeLog:
2016-08-09 Kugan Vivekanandarajah <kug...@linaro.org>
PR tree-optimization/61839
* tree-vrp.c (two_valued_val_range_p): New.
(simplify_stmt_using_ranges): Convert CST BINOP VAR where VAR is
two-valued to VAR == VAL1 ? (CST BINOP VAL1) : (CST BINOP VAL2).
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr61839.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr61839.c
index e69de29..1bb77d1 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr61839.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr61839.c
@@ -0,0 +1,40 @@
+/* PR tree-optimization/61839 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-vrp1" } */
+
+__attribute__ ((noinline))
+int foo ()
+{
+ int a = -1;
+ volatile unsigned b = 1U;
+ int c = 1;
+ c = (a + 972195718) >> (1LU <= b);
+ if (c == 486097858)
+ ;
+ else
+ __builtin_abort ();
+ return 0;
+}
+
+__attribute__ ((noinline))
+int bar ()
+{
+ int a = -1;
+ volatile unsigned b = 1U;
+ int c = 1;
+ c = (a + 972195718) >> (b ? 2 : 3);
+ if (c == 243048929)
+ ;
+ else
+ __builtin_abort ();
+ return 0;
+}
+
+int main ()
+{
+ foo ();
+ bar ();
+}
+
+/* { dg-final { scan-tree-dump-times "486097858 : 972195717" 1 "vrp1" } } */
+/* { dg-final { scan-tree-dump-times "243048929 : 121524464" 2 "vrp1" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 7c7ad91..d5e2fc3 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10004,6 +10004,27 @@ simplify_internal_call_using_ranges
(gimple_stmt_iterator *gsi, gimple *stmt)
return true;
}
+/* Return true if VAR is a two-valued variable. Set MIN and MAX when it is
+ true. Return false otherwise. */
+
+static bool
+two_valued_val_range_p (tree var, tree *min, tree *max)
+{
+ value_range *vr = get_value_range (var);
+ if (vr->type != VR_RANGE
+ || !range_int_cst_p (vr))
+ return false;
+ tree tmp
+ = int_const_binop (PLUS_EXPR,
+ vr->min,
+ build_int_cst_type (TREE_TYPE (var), 1));
+ if (0 != compare_values (tmp, vr->max))
+ return false;
+ *min = vr->min;
+ *max = vr->max;
+ return true;
+}
+
/* Simplify STMT using ranges if possible. */
static bool
@@ -10014,6 +10035,38 @@ simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree rhs2 = gimple_assign_rhs2 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ tree min, max;
+
+ /* Convert:
+ LHS = CST BINOP VAR
+ where VAR is two-valued.
+
+ To:
+ LHS = VAR == VAL1 ? (CST BINOP VAL1) : (CST BINOP VAL2) */
+
+ if (TREE_CODE_CLASS (rhs_code) == tcc_binary
+ && TREE_CODE (rhs1) == INTEGER_CST
+ && TREE_CODE (rhs2) == SSA_NAME
+ && has_single_use (rhs2)
+ && two_valued_val_range_p (rhs2, &min, &max))
+
+ {
+ tree cond = build2 (EQ_EXPR, TREE_TYPE (rhs2), rhs2, min);
+ tree new_rhs1 = int_const_binop (rhs_code, rhs1, min);
+ tree new_rhs2 = int_const_binop (rhs_code, rhs1, max);
+
+ if (new_rhs1 && new_rhs2)
+ {
+ gimple_assign_set_rhs_with_ops (gsi,
+ COND_EXPR, cond,
+ new_rhs1,
+ new_rhs2);
+ update_stmt (gsi_stmt (*gsi));
+ return true;
+ }
+ }
switch (rhs_code)
{
