| Message ID | 79a1d581-ed41-e72d-77f6-e73ae3d3968a@suse.cz |
|---|---|
| State | New |
| Headers | show |
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
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
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 >
> 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 >> >
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