Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Bill Schmidt]

> On Jul 15, 2016, at 2:24 AM, Richard Biener  
> wrote:
> 
> On Thu, Jul 14, 2016 at 6:10 PM, Martin Liška  wrote:
>> On 07/14/2016 01:21 PM, Richard Biener wrote:
>>> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška  wrote:
 On 07/13/2016 07:21 PM, Jeff Law wrote:
> Isn't that a code quality regression?  So instead shouldn't we be keeping 
> the same expectation, but xfailing the test?
> 
> jeff
 
 Hello.
 
 Disabling a pass before slsr makes the test to catch both opportunities.
 Is the patch fine?
>>> 
>>> No, this is still a code quality regression.  What happens is that for
>>> some reason we fail to sink for GCC 6.
>> 
>> So should I just mark the test-case as a xfail?
> 
> Leave it FAIL and open a bug.  We need to fix SLSR to handle the situation.

Please CC me on the bug (wschm...@gcc.gnu.org).

Thanks,
Bill

> 
> You can try going back to the point where the testcase was added and look at 
> the
> IL that it was supposed to test, on the GCC 6 branch we sink into one
> arm but not
> the other, on trunk we sink into both.  Iff the original IL was
> without any sinking
> then adding a testcase variant with sinking turned off might be good as well.
> 
> I'll also note that if we'd do these kind of tests as unit-tests we'd
> never notice
> that in real-world the testcase would have started failing due to
> previous passes
> messing up the IL.
> 
> Richard.
> 
>> M.
>> 
>>> 
>>> Richard.
>>> 
 Thanks,
 Martin
>> 
> 

Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Richard Biener]

On Thu, Jul 14, 2016 at 6:10 PM, Martin Liška  wrote:
> On 07/14/2016 01:21 PM, Richard Biener wrote:
>> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška  wrote:
>>> On 07/13/2016 07:21 PM, Jeff Law wrote:
 Isn't that a code quality regression?  So instead shouldn't we be keeping 
 the same expectation, but xfailing the test?

 jeff
>>>
>>> Hello.
>>>
>>> Disabling a pass before slsr makes the test to catch both opportunities.
>>> Is the patch fine?
>>
>> No, this is still a code quality regression.  What happens is that for
>> some reason we fail to sink for GCC 6.
>
> So should I just mark the test-case as a xfail?

Leave it FAIL and open a bug.  We need to fix SLSR to handle the situation.

You can try going back to the point where the testcase was added and look at the
IL that it was supposed to test, on the GCC 6 branch we sink into one
arm but not
the other, on trunk we sink into both.  Iff the original IL was
without any sinking
then adding a testcase variant with sinking turned off might be good as well.

I'll also note that if we'd do these kind of tests as unit-tests we'd
never notice
that in real-world the testcase would have started failing due to
previous passes
messing up the IL.

Richard.

> M.
>
>>
>> Richard.
>>
>>> Thanks,
>>> Martin
>

Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Martin Liška]

On 07/14/2016 01:21 PM, Richard Biener wrote:
> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška  wrote:
>> On 07/13/2016 07:21 PM, Jeff Law wrote:
>>> Isn't that a code quality regression?  So instead shouldn't we be keeping 
>>> the same expectation, but xfailing the test?
>>>
>>> jeff
>>
>> Hello.
>>
>> Disabling a pass before slsr makes the test to catch both opportunities.
>> Is the patch fine?
> 
> No, this is still a code quality regression.  What happens is that for
> some reason we fail to sink for GCC 6.

So should I just mark the test-case as a xfail?

M.

> 
> Richard.
> 
>> Thanks,
>> Martin

Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Richard Biener]

On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška  wrote:
> On 07/13/2016 07:21 PM, Jeff Law wrote:
>> Isn't that a code quality regression?  So instead shouldn't we be keeping 
>> the same expectation, but xfailing the test?
>>
>> jeff
>
> Hello.
>
> Disabling a pass before slsr makes the test to catch both opportunities.
> Is the patch fine?

No, this is still a code quality regression.  What happens is that for
some reason we fail to sink for GCC 6.

Richard.

> Thanks,
> Martin

Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Martin Liška]

On 07/13/2016 07:21 PM, Jeff Law wrote:
> Isn't that a code quality regression?  So instead shouldn't we be keeping the 
> same expectation, but xfailing the test?
> 
> jeff

Hello.

Disabling a pass before slsr makes the test to catch both opportunities.
Is the patch fine?

Thanks,
Martin
>From 59e3c47ca4fad03a8152776ad5100eed7b610883 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 14 Jul 2016 13:02:05 +0200
Subject: [PATCH] Amend dump expectation in slsr-8.c (PR
 tree-optimization/71490)

gcc/testsuite/ChangeLog:

2016-07-13  Martin Liska  

	* gcc.dg/tree-ssa/slsr-8.c: Disable -ftree-sink pass.
---
 gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
index 2bd60aa..557b798 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
@@ -1,7 +1,7 @@
 /* Verify straight-line strength reduction for simple pointer subtraction.  */
 
 /* { dg-do compile } */
-/* { dg-options "-O3 -fdump-tree-optimized" } */
+/* { dg-options "-O3 -fno-tree-sink -fdump-tree-optimized" } */
 
 int*
 f (int s, int *c)
-- 
2.8.4

Re: [PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)

[Jeff Law]

On 07/13/2016 08:47 AM, Martin Liška wrote:

Hello.

As mentioned in [1], one slsr transformation is gone, thus we need to change 
expected number
of multiplications.

Ready to be installed?
Thanks,
Martin

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71490#c5
Isn't that a code quality regression?  So instead shouldn't we be 
keeping the same expectation, but xfailing the test?


jeff