Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-03-01 Thread Richard Guenther
On Thu, Mar 1, 2012 at 3:45 PM, Bin.Cheng  wrote:
>>> Second point, as you said, PRE often get confused and moves compare
>>> EXPR far from jump statement. Could we rely on register re-materialize
>>> to handle this, or any other solution?
>>
>> Well, a simple kind of solution would be to preprocess the IL before
>> redundancy elimination and separate the predicate computation from
>> their uses and then as followup combine predicates back (tree forwprop
>> would do that, for example - even for multiple uses).  The question is
>> what you gain in the end.
>
> I realized there is no merit if compare EXPR is factored only for PRE pass.
>
>>
>>> I would like to learn more about this case, so do you have any opinion on
>>> how this should be fixed for now.
>>
>> The GIMPLE IL should be better here, especially if you consider that
>> we force away predicate computation that may trap for -fnon-call-exceptions
>> already.  So, simplifying the IL is still the way to go IMHO.  But as I said
>> above - it's a non-trivial task with possibly much fallout.
>>
> There is another benefit. Currently general compare EXPR is a dead case GCC
> can not handle in conditional const/copy propagation. It can be handled 
> properly
> after rewriting, since GIMPLE_COND only contains a predicate SSA_NAME.
> For example, redundant gimple generated for test case in pr38998:
>
> :
>  if (y_3(D) < 1.0e+1)
>    goto ;
>  else
>    goto ;
>
> :
>  D.4069_7 = cos (y_3(D));
>  if (y_3(D) < 1.0e+1)
>    goto ;
>  else
>    goto ;
>
> I do think these "non-canonical" compare EXPR might seed other issues.
>
> As for the fallout you mentioned, how about introduce a light-weight pass
> at the very end of middle end to propagate the compare EXPR back to
> GIMPLE_COND if the corresponding predicate SSA_NAME is down-safe
> only because it is used by GIMPLE_COND.
>
> So what do you think?

Well, I'm all for it, but the fallout is in the GIMPLE middle-end pieces.
It's just a lot of work ;)  And I'd rather start forcing the predicate
separation for VEC_COND_EXPRs and COND_EXPRs as they appear
on the RHS of gimple assigns.  That should be simpler and the fallout
should be less.

If you want to do the work I promise to review patches.

Richard.

> --
> Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-03-01 Thread Bin.Cheng
>> Second point, as you said, PRE often get confused and moves compare
>> EXPR far from jump statement. Could we rely on register re-materialize
>> to handle this, or any other solution?
>
> Well, a simple kind of solution would be to preprocess the IL before
> redundancy elimination and separate the predicate computation from
> their uses and then as followup combine predicates back (tree forwprop
> would do that, for example - even for multiple uses).  The question is
> what you gain in the end.

I realized there is no merit if compare EXPR is factored only for PRE pass.

>
>> I would like to learn more about this case, so do you have any opinion on
>> how this should be fixed for now.
>
> The GIMPLE IL should be better here, especially if you consider that
> we force away predicate computation that may trap for -fnon-call-exceptions
> already.  So, simplifying the IL is still the way to go IMHO.  But as I said
> above - it's a non-trivial task with possibly much fallout.
>
There is another benefit. Currently general compare EXPR is a dead case GCC
can not handle in conditional const/copy propagation. It can be handled properly
after rewriting, since GIMPLE_COND only contains a predicate SSA_NAME.
For example, redundant gimple generated for test case in pr38998:

:
  if (y_3(D) < 1.0e+1)
goto ;
  else
goto ;

:
  D.4069_7 = cos (y_3(D));
  if (y_3(D) < 1.0e+1)
goto ;
  else
goto ;

I do think these "non-canonical" compare EXPR might seed other issues.

As for the fallout you mentioned, how about introduce a light-weight pass
at the very end of middle end to propagate the compare EXPR back to
GIMPLE_COND if the corresponding predicate SSA_NAME is down-safe
only because it is used by GIMPLE_COND.

So what do you think?

-- 
Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-02-29 Thread Richard Guenther
On Wed, Feb 29, 2012 at 3:50 PM, Bin.Cheng  wrote:
> On Mon, Jan 2, 2012 at 10:54 PM, Richard Guenther
>  wrote:
>> On Mon, Jan 2, 2012 at 3:09 PM, Amker.Cheng  wrote:
>>> On Mon, Jan 2, 2012 at 9:37 PM, Richard Guenther
>>>  wrote:
>>>
 Well, with

 Index: gcc/tree-ssa-pre.c
 ===
 --- gcc/tree-ssa-pre.c  (revision 182784)
 +++ gcc/tree-ssa-pre.c  (working copy)
 @@ -4335,16 +4335,23 @@ eliminate (void)
             available value-numbers.  */
          else if (gimple_code (stmt) == GIMPLE_COND)
            {
 -             tree op0 = gimple_cond_lhs (stmt);
 -             tree op1 = gimple_cond_rhs (stmt);
 +             tree op[2];
              tree result;
 +             vn_nary_op_t nary;

 -             if (TREE_CODE (op0) == SSA_NAME)
 -               op0 = VN_INFO (op0)->valnum;
 -             if (TREE_CODE (op1) == SSA_NAME)
 -               op1 = VN_INFO (op1)->valnum;
 +             op[0] = gimple_cond_lhs (stmt);
 +             op[1] = gimple_cond_rhs (stmt);
 +             if (TREE_CODE (op[0]) == SSA_NAME)
 +               op[0] = VN_INFO (op[0])->valnum;
 +             if (TREE_CODE (op[1]) == SSA_NAME)
 +               op[1] = VN_INFO (op[1])->valnum;
              result = fold_binary (gimple_cond_code (stmt), 
 boolean_type_node,
 -                                   op0, op1);
 +                                   op[0], op[1]);
 +             if (!result)
 +               result = vn_nary_op_lookup_pieces (2, gimple_cond_code 
 (stmt),
 +                                                  boolean_type_node,
 +                                                  op, &nary);
 +
              if (result && TREE_CODE (result) == INTEGER_CST)
                {
                  if (integer_zerop (result))
 @@ -4354,6 +4361,13 @@ eliminate (void)
                  update_stmt (stmt);
                  todo = TODO_cleanup_cfg;
                }
 +             else if (result && TREE_CODE (result) == SSA_NAME)
 +               {
 +                 gimple_cond_set_code (stmt, NE_EXPR);
 +                 gimple_cond_set_lhs (stmt, result);
 +                 gimple_cond_set_rhs (stmt, boolean_false_node);
 +                 update_stmt (stmt);
 +               }
            }
          /* Visit indirect calls and turn them into direct calls if
             possible.  */

 you get the CSE (too simple patch, you need to check leaders properly).
 You can then add similar lookups for an inverted conditional.
>>>
>>> Thanks for your explanation. On shortcoming of this method is that it
>>> cannot find/take cond_expr(and the implicitly defined variable) as the
>>> leader in pre. I guess this is why you said it can handle a subset of all
>>> cases in previous message?
>>
>> Yes.  It won't handle
>>
>>  if (x > 1)
>>   ...
>>  tem = x > 1;
>>
>> or
>>
>>  if (x > 1)
>>   ...
>>  if (x > 1)
>>
>> though maybe we could teach PRE to do the insertion by properly
>> putting x > 1 into EXP_GEN in compute_avail (but not into AVAIL_OUT).
>> Not sure about this though.  Currently we don't do anything to
>> GIMPLE_COND operands (which seems odd anyway, we should
>> at least add the operands to EXP_GEN).
> I spent some time on this issue again.
> Yes, we can insert compare EXPR in EXP_GEN(but not AVAIL_OUT).
> We also can teach PRE to insert the recorded compare EXPR in previous
> mentioned special cases to solve the problem.
> But, this way it's not different from factoring compare EXPR out of
> GIMPLE_COND, since insertion/elimination itself is a kind of rewriting.
> Thus I think the problem should be fixed by rewriting/factoring compare
> EXPR out of GIMPLE_COND.

Yes, see http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02098.html
unfortunately it is a non-trivial task ;)

> Second point, as you said, PRE often get confused and moves compare
> EXPR far from jump statement. Could we rely on register re-materialize
> to handle this, or any other solution?

Well, a simple kind of solution would be to preprocess the IL before
redundancy elimination and separate the predicate computation from
their uses and then as followup combine predicates back (tree forwprop
would do that, for example - even for multiple uses).  The question is
what you gain in the end.

> I would like to learn more about this case, so do you have any opinion on
> how this should be fixed for now.

The GIMPLE IL should be better here, especially if you consider that
we force away predicate computation that may trap for -fnon-call-exceptions
already.  So, simplifying the IL is still the way to go IMHO.  But as I said
above - it's a non-trivial task with possibly much fallout.

Richard.

> Thanks
> --
> Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-02-29 Thread Bin.Cheng
On Mon, Jan 2, 2012 at 10:54 PM, Richard Guenther
 wrote:
> On Mon, Jan 2, 2012 at 3:09 PM, Amker.Cheng  wrote:
>> On Mon, Jan 2, 2012 at 9:37 PM, Richard Guenther
>>  wrote:
>>
>>> Well, with
>>>
>>> Index: gcc/tree-ssa-pre.c
>>> ===
>>> --- gcc/tree-ssa-pre.c  (revision 182784)
>>> +++ gcc/tree-ssa-pre.c  (working copy)
>>> @@ -4335,16 +4335,23 @@ eliminate (void)
>>>             available value-numbers.  */
>>>          else if (gimple_code (stmt) == GIMPLE_COND)
>>>            {
>>> -             tree op0 = gimple_cond_lhs (stmt);
>>> -             tree op1 = gimple_cond_rhs (stmt);
>>> +             tree op[2];
>>>              tree result;
>>> +             vn_nary_op_t nary;
>>>
>>> -             if (TREE_CODE (op0) == SSA_NAME)
>>> -               op0 = VN_INFO (op0)->valnum;
>>> -             if (TREE_CODE (op1) == SSA_NAME)
>>> -               op1 = VN_INFO (op1)->valnum;
>>> +             op[0] = gimple_cond_lhs (stmt);
>>> +             op[1] = gimple_cond_rhs (stmt);
>>> +             if (TREE_CODE (op[0]) == SSA_NAME)
>>> +               op[0] = VN_INFO (op[0])->valnum;
>>> +             if (TREE_CODE (op[1]) == SSA_NAME)
>>> +               op[1] = VN_INFO (op[1])->valnum;
>>>              result = fold_binary (gimple_cond_code (stmt), 
>>> boolean_type_node,
>>> -                                   op0, op1);
>>> +                                   op[0], op[1]);
>>> +             if (!result)
>>> +               result = vn_nary_op_lookup_pieces (2, gimple_cond_code 
>>> (stmt),
>>> +                                                  boolean_type_node,
>>> +                                                  op, &nary);
>>> +
>>>              if (result && TREE_CODE (result) == INTEGER_CST)
>>>                {
>>>                  if (integer_zerop (result))
>>> @@ -4354,6 +4361,13 @@ eliminate (void)
>>>                  update_stmt (stmt);
>>>                  todo = TODO_cleanup_cfg;
>>>                }
>>> +             else if (result && TREE_CODE (result) == SSA_NAME)
>>> +               {
>>> +                 gimple_cond_set_code (stmt, NE_EXPR);
>>> +                 gimple_cond_set_lhs (stmt, result);
>>> +                 gimple_cond_set_rhs (stmt, boolean_false_node);
>>> +                 update_stmt (stmt);
>>> +               }
>>>            }
>>>          /* Visit indirect calls and turn them into direct calls if
>>>             possible.  */
>>>
>>> you get the CSE (too simple patch, you need to check leaders properly).
>>> You can then add similar lookups for an inverted conditional.
>>
>> Thanks for your explanation. On shortcoming of this method is that it
>> cannot find/take cond_expr(and the implicitly defined variable) as the
>> leader in pre. I guess this is why you said it can handle a subset of all
>> cases in previous message?
>
> Yes.  It won't handle
>
>  if (x > 1)
>   ...
>  tem = x > 1;
>
> or
>
>  if (x > 1)
>   ...
>  if (x > 1)
>
> though maybe we could teach PRE to do the insertion by properly
> putting x > 1 into EXP_GEN in compute_avail (but not into AVAIL_OUT).
> Not sure about this though.  Currently we don't do anything to
> GIMPLE_COND operands (which seems odd anyway, we should
> at least add the operands to EXP_GEN).
I spent some time on this issue again.
Yes, we can insert compare EXPR in EXP_GEN(but not AVAIL_OUT).
We also can teach PRE to insert the recorded compare EXPR in previous
mentioned special cases to solve the problem.
But, this way it's not different from factoring compare EXPR out of
GIMPLE_COND, since insertion/elimination itself is a kind of rewriting.
Thus I think the problem should be fixed by rewriting/factoring compare
EXPR out of GIMPLE_COND.

Second point, as you said, PRE often get confused and moves compare
EXPR far from jump statement. Could we rely on register re-materialize
to handle this, or any other solution?

I would like to learn more about this case, so do you have any opinion on
how this should be fixed for now.

Thanks
-- 
Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-03 Thread Amker.Cheng
On Mon, Jan 2, 2012 at 10:54 PM, Richard Guenther
 wrote:

> Yes.  It won't handle
>
>  if (x > 1)
>   ...
>  tem = x > 1;
>
> or
>
>  if (x > 1)
>   ...
>  if (x > 1)
>
> though maybe we could teach PRE to do the insertion by properly
> putting x > 1 into EXP_GEN in compute_avail (but not into AVAIL_OUT).
> Not sure about this though.  Currently we don't do anything to
> GIMPLE_COND operands (which seems odd anyway, we should
> at least add the operands to EXP_GEN).

I did an experiment which shows by setting cond_expr in EXP_GEN
properly, PRE could insert expression in following case:

//necessary declaration of variable a/b/g
int tmp;
if (x_cond)
  tmp = a > 2;
else
  tmp = b;
if (a > 2)
  g = tmp;

But the problem you mention : "PRE separates conditional expression
and jump to far" still exists in this kind of cases.
Now I doubt the benefit to make PRE handle cond_expr, because in back
end, machines normally have only one flag to store the result.

And for other cases like:
if (a > 2)
...
if (a > 2)

Current logic of insertion(in do_regular_insertion) simply won't
insert expression before the first GIMPLE_COND statement, because it
only considers basic blocks have multiple predecessors and the
expression are partial redundant.
Anyway I think this can be done by implementing new insertion strategy
for GIMPLE_COND.

-- 
Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-02 Thread Richard Guenther
On Mon, Jan 2, 2012 at 3:09 PM, Amker.Cheng  wrote:
> On Mon, Jan 2, 2012 at 9:37 PM, Richard Guenther
>  wrote:
>
>> Well, with
>>
>> Index: gcc/tree-ssa-pre.c
>> ===
>> --- gcc/tree-ssa-pre.c  (revision 182784)
>> +++ gcc/tree-ssa-pre.c  (working copy)
>> @@ -4335,16 +4335,23 @@ eliminate (void)
>>             available value-numbers.  */
>>          else if (gimple_code (stmt) == GIMPLE_COND)
>>            {
>> -             tree op0 = gimple_cond_lhs (stmt);
>> -             tree op1 = gimple_cond_rhs (stmt);
>> +             tree op[2];
>>              tree result;
>> +             vn_nary_op_t nary;
>>
>> -             if (TREE_CODE (op0) == SSA_NAME)
>> -               op0 = VN_INFO (op0)->valnum;
>> -             if (TREE_CODE (op1) == SSA_NAME)
>> -               op1 = VN_INFO (op1)->valnum;
>> +             op[0] = gimple_cond_lhs (stmt);
>> +             op[1] = gimple_cond_rhs (stmt);
>> +             if (TREE_CODE (op[0]) == SSA_NAME)
>> +               op[0] = VN_INFO (op[0])->valnum;
>> +             if (TREE_CODE (op[1]) == SSA_NAME)
>> +               op[1] = VN_INFO (op[1])->valnum;
>>              result = fold_binary (gimple_cond_code (stmt), 
>> boolean_type_node,
>> -                                   op0, op1);
>> +                                   op[0], op[1]);
>> +             if (!result)
>> +               result = vn_nary_op_lookup_pieces (2, gimple_cond_code 
>> (stmt),
>> +                                                  boolean_type_node,
>> +                                                  op, &nary);
>> +
>>              if (result && TREE_CODE (result) == INTEGER_CST)
>>                {
>>                  if (integer_zerop (result))
>> @@ -4354,6 +4361,13 @@ eliminate (void)
>>                  update_stmt (stmt);
>>                  todo = TODO_cleanup_cfg;
>>                }
>> +             else if (result && TREE_CODE (result) == SSA_NAME)
>> +               {
>> +                 gimple_cond_set_code (stmt, NE_EXPR);
>> +                 gimple_cond_set_lhs (stmt, result);
>> +                 gimple_cond_set_rhs (stmt, boolean_false_node);
>> +                 update_stmt (stmt);
>> +               }
>>            }
>>          /* Visit indirect calls and turn them into direct calls if
>>             possible.  */
>>
>> you get the CSE (too simple patch, you need to check leaders properly).
>> You can then add similar lookups for an inverted conditional.
>
> Thanks for your explanation. On shortcoming of this method is that it
> cannot find/take cond_expr(and the implicitly defined variable) as the
> leader in pre. I guess this is why you said it can handle a subset of all
> cases in previous message?

Yes.  It won't handle

  if (x > 1)
   ...
  tem = x > 1;

or

  if (x > 1)
   ...
  if (x > 1)

though maybe we could teach PRE to do the insertion by properly
putting x > 1 into EXP_GEN in compute_avail (but not into AVAIL_OUT).
Not sure about this though.  Currently we don't do anything to
GIMPLE_COND operands (which seems odd anyway, we should
at least add the operands to EXP_GEN).

> on the other hand, I like this method, given the simplicity especially. :)

Yeah.

> --
> Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-02 Thread Amker.Cheng
On Mon, Jan 2, 2012 at 9:37 PM, Richard Guenther
 wrote:

> Well, with
>
> Index: gcc/tree-ssa-pre.c
> ===
> --- gcc/tree-ssa-pre.c  (revision 182784)
> +++ gcc/tree-ssa-pre.c  (working copy)
> @@ -4335,16 +4335,23 @@ eliminate (void)
>             available value-numbers.  */
>          else if (gimple_code (stmt) == GIMPLE_COND)
>            {
> -             tree op0 = gimple_cond_lhs (stmt);
> -             tree op1 = gimple_cond_rhs (stmt);
> +             tree op[2];
>              tree result;
> +             vn_nary_op_t nary;
>
> -             if (TREE_CODE (op0) == SSA_NAME)
> -               op0 = VN_INFO (op0)->valnum;
> -             if (TREE_CODE (op1) == SSA_NAME)
> -               op1 = VN_INFO (op1)->valnum;
> +             op[0] = gimple_cond_lhs (stmt);
> +             op[1] = gimple_cond_rhs (stmt);
> +             if (TREE_CODE (op[0]) == SSA_NAME)
> +               op[0] = VN_INFO (op[0])->valnum;
> +             if (TREE_CODE (op[1]) == SSA_NAME)
> +               op[1] = VN_INFO (op[1])->valnum;
>              result = fold_binary (gimple_cond_code (stmt), boolean_type_node,
> -                                   op0, op1);
> +                                   op[0], op[1]);
> +             if (!result)
> +               result = vn_nary_op_lookup_pieces (2, gimple_cond_code (stmt),
> +                                                  boolean_type_node,
> +                                                  op, &nary);
> +
>              if (result && TREE_CODE (result) == INTEGER_CST)
>                {
>                  if (integer_zerop (result))
> @@ -4354,6 +4361,13 @@ eliminate (void)
>                  update_stmt (stmt);
>                  todo = TODO_cleanup_cfg;
>                }
> +             else if (result && TREE_CODE (result) == SSA_NAME)
> +               {
> +                 gimple_cond_set_code (stmt, NE_EXPR);
> +                 gimple_cond_set_lhs (stmt, result);
> +                 gimple_cond_set_rhs (stmt, boolean_false_node);
> +                 update_stmt (stmt);
> +               }
>            }
>          /* Visit indirect calls and turn them into direct calls if
>             possible.  */
>
> you get the CSE (too simple patch, you need to check leaders properly).
> You can then add similar lookups for an inverted conditional.

Thanks for your explanation. On shortcoming of this method is that it
cannot find/take cond_expr(and the implicitly defined variable) as the
leader in pre. I guess this is why you said it can handle a subset of all
cases in previous message?

on the other hand, I like this method, given the simplicity especially. :)

-- 
Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-02 Thread Richard Guenther
On Mon, Jan 2, 2012 at 2:11 PM, Amker.Cheng  wrote:
> Thanks Richard,
>
> On Mon, Jan 2, 2012 at 8:33 PM, Richard Guenther
>  wrote:
>>
>> I've previously worked on changing GIMPLE_COND to no longer embed
>> the comparison but carry a predicate SSA_NAME only (this is effectively
>> what you do as pre-processing before SCCVN).  It had some non-trivial
>> fallout (for example PRE get's quite confused and ends up separating
>> conditionals and jumps too far ...) so I didn't finish it.
> Here changing GIMPLE_COND to no longer embed the comparison,
> do you mean this only in fre/pre passes or in common?
> If only in fre/pre passes, when and how these changed GIMPLE_COND
> be changed back to normal ones?
> If in common, won't this affects passes working on GIMPLE_COND, like
> tree-ssa-forwprop.c?

Everywhere.  I posted a patch early last year and changed COND_EXPRs
on the RHS of an assignment later.

>>
>> A subset of all cases can be catched by simply looking up the
>> N-ary at eliminate () time and re-writing the GIMPLE_COND to use
>> the predicate - which might not actually be beneficial (but forwprop
>> will undo not beneficial cases - hopefully).
>>
>> In the end I'd rather go the way changing the GIMPLE IL to not
>> embed the comparison in the GIMPLE_COND - that reduces
>> the amount of redundant way we can express the same thing.
> Will you try to handle the reversion comparison case as mentioned
> in my previous message? I guess this needs both sccvn and fre/pre's
> work. It would be great to hear your thoughts on this.

Well, with

Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c  (revision 182784)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -4335,16 +4335,23 @@ eliminate (void)
 available value-numbers.  */
  else if (gimple_code (stmt) == GIMPLE_COND)
{
- tree op0 = gimple_cond_lhs (stmt);
- tree op1 = gimple_cond_rhs (stmt);
+ tree op[2];
  tree result;
+ vn_nary_op_t nary;

- if (TREE_CODE (op0) == SSA_NAME)
-   op0 = VN_INFO (op0)->valnum;
- if (TREE_CODE (op1) == SSA_NAME)
-   op1 = VN_INFO (op1)->valnum;
+ op[0] = gimple_cond_lhs (stmt);
+ op[1] = gimple_cond_rhs (stmt);
+ if (TREE_CODE (op[0]) == SSA_NAME)
+   op[0] = VN_INFO (op[0])->valnum;
+ if (TREE_CODE (op[1]) == SSA_NAME)
+   op[1] = VN_INFO (op[1])->valnum;
  result = fold_binary (gimple_cond_code (stmt), boolean_type_node,
-   op0, op1);
+   op[0], op[1]);
+ if (!result)
+   result = vn_nary_op_lookup_pieces (2, gimple_cond_code (stmt),
+  boolean_type_node,
+  op, &nary);
+
  if (result && TREE_CODE (result) == INTEGER_CST)
{
  if (integer_zerop (result))
@@ -4354,6 +4361,13 @@ eliminate (void)
  update_stmt (stmt);
  todo = TODO_cleanup_cfg;
}
+ else if (result && TREE_CODE (result) == SSA_NAME)
+   {
+ gimple_cond_set_code (stmt, NE_EXPR);
+ gimple_cond_set_lhs (stmt, result);
+ gimple_cond_set_rhs (stmt, boolean_false_node);
+ update_stmt (stmt);
+   }
}
  /* Visit indirect calls and turn them into direct calls if
 possible.  */

you get the CSE (too simple patch, you need to check leaders properly).
You can then add similar lookups for an inverted conditional.

Richard.

> Thanks
>
> --
> Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-02 Thread Amker.Cheng
Thanks Richard,

On Mon, Jan 2, 2012 at 8:33 PM, Richard Guenther
 wrote:
>
> I've previously worked on changing GIMPLE_COND to no longer embed
> the comparison but carry a predicate SSA_NAME only (this is effectively
> what you do as pre-processing before SCCVN).  It had some non-trivial
> fallout (for example PRE get's quite confused and ends up separating
> conditionals and jumps too far ...) so I didn't finish it.
Here changing GIMPLE_COND to no longer embed the comparison,
do you mean this only in fre/pre passes or in common?
If only in fre/pre passes, when and how these changed GIMPLE_COND
be changed back to normal ones?
If in common, won't this affects passes working on GIMPLE_COND, like
tree-ssa-forwprop.c?

>
> A subset of all cases can be catched by simply looking up the
> N-ary at eliminate () time and re-writing the GIMPLE_COND to use
> the predicate - which might not actually be beneficial (but forwprop
> will undo not beneficial cases - hopefully).
>
> In the end I'd rather go the way changing the GIMPLE IL to not
> embed the comparison in the GIMPLE_COND - that reduces
> the amount of redundant way we can express the same thing.
Will you try to handle the reversion comparison case as mentioned
in my previous message? I guess this needs both sccvn and fre/pre's
work. It would be great to hear your thoughts on this.

Thanks

-- 
Best Regards.


Re: RFC: Handle conditional expression in sccvn/fre/pre

2012-01-02 Thread Richard Guenther
On Mon, Jan 2, 2012 at 12:37 PM, Amker.Cheng  wrote:
> Hi,
> Since SCCVN operates on SSA graph instead of the control flow graph
> for the sake of efficiency,
> it does not handle or value number the conditional expression of
> GIMPLE_COND statement.
> As a result, FRE/PRE does not simplify conditional expression, as
> reported in bug 30997.
>
> Since it would be complicate and difficult to process conditional
> expression in currently SCCVN
> algorithm, how about following method?
>
> STEP1  Before starting FRE/PRE, we can factor out the conditional
> expression, like change following
> codes:
> 
> if (cond_expr)
>  goto lable_a
> else
>  goto label_b
>
> into codes:
> 
> tmp = cond_expr
> if (tmp == 1)
>  goto label_a
> else
>  goto label_b
>
> STEP2  Let SCCVN/FRE/PRE do its job on value numbering cond_expr and
> redundancy elimination;
> STEP3  After FRE/PRE, for those "tmp=cond_expr" not used in any
> redundancy elimination,
> we can forward it to the corresponding GIMPLE_COND statement, just
> like tree-ssa-forwprop.c.
>
> In this way, the conditional expression will be handled as other
> expressions and no
> redundant assignment generated.
> Most important,this does not affect the current implementation of 
> SCCVN/FRE/PRE.
>
> The only problem is the method cannot work on reversion of conditional
> expression.
> For example:
> 
> x = a > 2;
> if (a<=2)
>  goto label_a
> else
>  goto lable_b
> could be optimized as:
> 
> x = a > 2
> if (x == 0)
>  goto label_a
> else
>  goto label_b
> I have worked a draft patch to do the work and would like to hear your
> comments on this.

I've previously worked on changing GIMPLE_COND to no longer embed
the comparison but carry a predicate SSA_NAME only (this is effectively
what you do as pre-processing before SCCVN).  It had some non-trivial
fallout (for example PRE get's quite confused and ends up separating
conditionals and jumps too far ...) so I didn't finish it.

A subset of all cases can be catched by simply looking up the
N-ary at eliminate () time and re-writing the GIMPLE_COND to use
the predicate - which might not actually be beneficial (but forwprop
will undo not beneficial cases - hopefully).

In the end I'd rather go the way changing the GIMPLE IL to not
embed the comparison in the GIMPLE_COND - that reduces
the amount of redundant way we can express the same thing.

Richard.

> Thanks very much.
> --
> Best Regards.