[PATCH] Amend dump expectation in slsr-8.c (PR, tree-optimization/71490)
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 >From ac130165f6c8166c28227fec2a6fa3afbccadb27 Mon Sep 17 00:00:00 2001 From: marxin 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 * 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)
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)
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)
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)
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)
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)
> 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 >> >