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

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

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

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

[-- Attachment #2: 0001-Amend-dump-expectation-in-slsr-8.c-PR-tree-optimizat.patch --]
[-- Type: text/x-patch, Size: 1088 bytes --]

From ac130165f6c8166c28227fec2a6fa3afbccadb27 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 13 Jul 2016 16:39:28 +0200
Subject: [PATCH] Amend dump expectation in slsr-8.c (PR
 tree-optimization/71490)

gcc/testsuite/ChangeLog:

2016-07-13  Martin Liska  <mliska@suse.cz>

	* gcc.dg/tree-ssa/slsr-8.c: Amend dump expectation.
---
 gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
index 2bd60aa..47b644b 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-8.c
@@ -18,6 +18,6 @@ f (int s, int *c)
 }
 
 /* There are 4 ' * ' instances in the decls (since "int * iftmp.0;" is
-   added), 1 parm, 2 in the code.  The second one in the code can be
+   added), 1 parm, 4 in the code.  The second one in the code can be
    a widening mult.  */
-/* { dg-final { scan-tree-dump-times " w?\\* " 7 "optimized" } } */
+/* { dg-final { scan-tree-dump-times " w?\\* " 9 "optimized" } } */
-- 
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

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

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

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

[-- Attachment #2: 0001-Amend-dump-expectation-in-slsr-8.c-PR-tree-optimizat-v2.patch --]
[-- Type: text/x-patch, Size: 935 bytes --]

From 59e3c47ca4fad03a8152776ad5100eed7b610883 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
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  <mliska@suse.cz>

	* 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)

[Richard Biener]

On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška <mliska@suse.cz> 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/14/2016 01:21 PM, Richard Biener wrote:
> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška <mliska@suse.cz> 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 6:10 PM, Martin Liška <mliska@suse.cz> wrote:
> On 07/14/2016 01:21 PM, Richard Biener wrote:
>> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška <mliska@suse.cz> 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)

[Bill Schmidt]

> On Jul 15, 2016, at 2:24 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Jul 14, 2016 at 6:10 PM, Martin Liška <mliska@suse.cz> wrote:
>> On 07/14/2016 01:21 PM, Richard Biener wrote:
>>> On Thu, Jul 14, 2016 at 1:06 PM, Martin Liška <mliska@suse.cz> 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 (wschmidt@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
>> 
>