[RFC][PR61839]Convert CST BINOP COND_EXPR to COND_EXPR ? (CST BINOP 1) : (CST BINOP 0)

[Kugan Vivekanandarajah]

Ping?

https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00987.html

Thanks,
Kugan

On 12 August 2016 at 13:19, kugan <kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>
> On 11/08/16 20:04, Richard Biener wrote:
>>
>> On Thu, Aug 11, 2016 at 6:11 AM, kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>
>
> [SNIP]
>
>>
>> +two_valued_val_range_p (tree var, tree *a, tree *b)
>> +{
>> +  value_range *vr = get_value_range (var);
>> +  if ((vr->type != VR_RANGE
>> +       && vr->type != VR_ANTI_RANGE)
>> +      || !range_int_cst_p (vr))
>> +    return false;
>>
>> range_int_cst_p checks for vr->type == VR_RANGE so the anti-range handling
>> doesn't ever trigger - which means you should add a testcase for it as
>> well.
>
>
> Fixed it. I have also added a testcase.
>
>>
>>
>> +    {
>> +      *a = TYPE_MIN_VALUE (TREE_TYPE (var));
>> +      *b = TYPE_MAX_VALUE (TREE_TYPE (var));
>>
>> note that for pointer types this doesn't work, please also use
>> vrp_val_min/max for
>> consistency.  I think you want to add a INTEGRAL_TYPE_P (TREE_TYPE (var))
>> to the guard of two_valued_val_range_p.
>>
>> +      /* First canonicalize to simplify tests.  */
>> +      if (commutative_tree_code (rhs_code)
>> +         && TREE_CODE (rhs2) == INTEGER_CST)
>> +       std::swap (rhs1, rhs2);
>>
>> note that this doesn't really address my comment - if you just want to
>> handle
>> commutative ops then simply look for the constant in the second place
>> rather
>> than the first which is the canonical operand order.  But even for
>> non-commutative
>> operations we might want to apply this optimization - and then for both
>> cases,
>> rhs1 or rhs2 being constant.  Like x / 5 and 5 / x.
>>
>> Note that you can rely on int_const_binop returning NULL_TREE for
>> "invalid"
>> ops like x % 0 or x / 0, so no need to explicitely guard this here.
>
>
> Sorry, I misunderstood you. I have changed it now. I also added test-case to
> check this.
>
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions. Is this OK for trunk now?
>
> Thanks,
> Kugan
>
> gcc/testsuite/ChangeLog:
>
> 2016-08-12  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         PR tree-optimization/61839
>         * gcc.dg/tree-ssa/pr61839_1.c: New test.
>         * gcc.dg/tree-ssa/pr61839_2.c: New test.
>         * gcc.dg/tree-ssa/pr61839_3.c: New test.
>         * gcc.dg/tree-ssa/pr61839_4.c: New test.
>
> gcc/ChangeLog:
>
> 2016-08-12  Kugan Vivekanandarajah  <kuganv@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).
>         Also Convert VAR BINOP CST where VAR is two-valued to
>         VAR == VAL1 ? (VAL1 BINOP CST) : (VAL2 BINOP CST).