On 10/08/16 08:51, kugan wrote:
Hi Jakub,

On 10/08/16 07:55, Jakub Jelinek wrote:
On Wed, Aug 10, 2016 at 07:51:08AM +1000, kugan wrote:
On 10/08/16 07:46, Jakub Jelinek wrote:
On Wed, Aug 10, 2016 at 07:42:25AM +1000, kugan wrote:
There was no new regression while testing. I also moved the testcase from
gcc.dg/torture/pr72835.c to gcc.dg/tree-ssa/pr72835.c. Is this OK for trunk?

This looks strange.  The tree-ssa-reassoc.c code has been trying to never
reuse SSA_NAMEs if they would hold a different value.
So there should be no resetting of flow sensitive info needed.

We are not reusing but, if you see the example change in reassoc:

-  _5 = -_4;
-  _6 = _2 * _5;
+  _5 = _4;
+  _6 = _5 * _2;

_5 and _6 will now have different value ranges because they compute
different values. Therefore I think we should reset (?).

No.  We should not have reused _5 and _6 for the different values.
It is not harmful just for the value ranges, but also for debug info.

I see it now. The problem is we are just looking at (-1) being in the
ops list for passing changed to rewrite_expr_tree in the case of
multiplication by negate.  If we have combined (-1), as in the testcase,
we will not have the (-1) and will pass changed=false to rewrite_expr_tree.

We should set changed based on what happens in try_special_add_to_ops.
Attached patch does this. Bootstrap and regression testing are ongoing.
Is this OK for trunk if there is no regression.


This patch unfortunately does not handle all the cases. I am revising it. Sorry for the noise.

Thanks,
Kugan

Reply via email to