[Bug middle-end/53676] New: [4.7/4.8 regression] constant folding regression, shown as slowdown as measured by Adobe's C++Benchmark

[Bug middle-end/53676] New: [4.7/4.8 regression] constant folding regression, shown as slowdown as measured by Adobe's C++Benchmark

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

             Bug #: 53676
           Summary: [4.7/4.8 regression] constant folding regression,
                    shown as slowdown as measured by Adobe's C++Benchmark
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: matt@use.net


The attached ZIP contains two binaries and preprocessor output files, based on
a subset of a test in Adobe's C++Benchmark suite. All GCC versions I tested
prior to 4.7 do the folding correctly. I tried using Ofast and other tricks to
workaround this new regression to no avail.

-rw-r--r-- 1 mhargett users 143573 Jun 14 15:31 int8_folding_test.gcc47.i
mhargett@chert:~/src/C++Benchmarks-gcc47-google-fast$ ./int8_folding_test-gcc46
./int8_folding_test-gcc46 

test     description   absolute   operations   ratio with
number                 time       per second   test0

 0 "int8_t constant"   0.00 sec     inf M     -nan

Total absolute time for int8_t simple constant folding: 0.00 sec
mhargett@chert:~/src/C++Benchmarks-gcc47-google-fast$ ./int8_folding_test-gcc47
./int8_folding_test-gcc47 

test     description   absolute   operations   ratio with
number                 time       per second   test0

 0 "int8_t constant"   0.69 sec   2318.84 M     1.00

Total absolute time for int8_t simple constant folding: 0.69 sec

[Bug middle-end/53676] [4.7/4.8 regression] constant folding regression, shown as slowdown as measured by Adobe's C++Benchmark

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #1 from Matt Hargett <matt at use dot net> 2012-06-14 22:48:49 UTC ---
Created attachment 27622
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27622
ZIP containing pre-processed source and binaries that demonstrate the const
folding regression

[Bug middle-end/53676] [4.7/4.8 regression] constant folding regression, shown as slowdown as measured by Adobe's C++Benchmark

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #2 from Matt Hargett <matt at use dot net> 2012-06-14 23:00:33 UTC ---
I forgot to mention -- it's the same result on all types, both signed and
unsigned. the int8_t case is (hopefully) representative of the root cause for
all/most of them.

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-06-14
   Target Milestone|---                         |4.7.2
            Summary|[4.7/4.8 regression]        |[4.7/4.8 regression] empty
                   |constant folding            |loop is not always removed
                   |regression, shown as        |now
                   |slowdown as measured by     |
                   |Adobe's C++Benchmark        |
     Ever Confirmed|0                           |1
           Severity|major                       |normal

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-06-14 23:13:14 UTC ---
Empty loop could get rid of the loop in previous versions of GCC.  That was
replaced with cdce but that did not get rid of the empty loop.  This is a minor
issue really because this benchmark is just making sure you got rid of an semi
empty loop.

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at redhat dot com

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2012-06-15 00:01:41 UTC ---
It is caused by revision 176072:

http://gcc.gnu.org/ml/gcc-cvs/2011-07/msg00337.html

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-19 13:18:19 UTC ---
First of all confirmed.  We rely on SCCP to compute the overall loop effect
which does not happen anymore.  The loops are different - 4.6 has

<bb 14>:
  # result_81 = PHI <result_44(13), 0(21)>
  # n_78 = PHI <n_46(13), 0(21)>
  result.7_42 = (unsigned char) result_81;
  D.7248_43 = result.7_42 + 2;
  result_44 = (signed char) D.7248_43;
  n_46 = n_78 + 1;
  if (n_46 != 8000)
    goto <bb 13>;
  else
    goto <bb 15>;

but with 4.7 we see

<bb 16>:
  # result_76 = PHI <result_50(15), 0(22)>
  # n_77 = PHI <n_52(15), 0(22)>
  D.7583_48 = (int) result_76;
  D.7582_49 = D.7583_48 + 2;
  result_50 = (signed char) D.7582_49;
  n_52 = n_77 + 1;
  if (n_52 != 8000)
    goto <bb 15>;
  else
    goto <bb 17>;

thus 4.6 performed the premature shortening optimization that 4.7 no longer
performs.

Testcase that nobody handles because it has the operation explicitely in int:

int main()
{
  int i;
  signed char result = 0;
  for (i = 0; i != 8000; ++i)
    {
      int tem = result;
      tem = tem + 2;
      result = tem;
    }
  if (__builtin_abs ((int)(signed char)((unsigned char ) result + 128)) != 0)
    __builtin_abort ();
  return 0;
}

Proper action could be to implement that shortening inside forwprop or
to teach the trick to SCEV analysis.

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-25 08:17:38 UTC ---
I will look at this.

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-25 13:39:29 UTC ---
In SCEV analysis we'd need to detect the pattern feeding the truncation

  tem_3 = (int) result_2;
  tem_4 = tem_3 + 2;
  result_5 = (signed char) tem_4;

in interpret_rhs_expr - otherwise we ask for the evolution of tem_4 which
we cannot express properly because tem_3's chrec wraps.  So we'd need to
handle result_5 = (signed char) ((int) result_2 + 2) as if it were done
without the intermediate tem_4.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.3, 4.8.0
            Summary|[4.7/4.8 regression] empty  |[4.7 regression] empty loop
                   |loop is not always removed  |is not always removed now
                   |now                         |
      Known to fail|                            |4.7.1

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-27 11:32:34 UTC ---
Author: rguenth
Date: Wed Jun 27 11:32:30 2012
New Revision: 189013

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189013
Log:
2012-06-27  Richard Guenther  <rguenther@suse.de>

    PR middle-end/53676
    * tree-chrec.c (chrec_convert_1): Represent truncation to
    a type with undefined overflow as truncation to an unsigned
    type converted to the type with undefined overflow.
    * tree-scalar-evolution.c (interpret_rhs_expr): For computing
    the scalar evolution of a truncated widened operation avoid
    looking at the non-existing evolution of the widened operation
    result.

    * gcc.dg/tree-ssa/scev-6.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/scev-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-chrec.c
    trunk/gcc/tree-scalar-evolution.c

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-27 11:33:12 UTC ---
Fixed for trunk.

[Bug middle-end/53676] [4.7/4.8 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-27 11:32:34 UTC ---
Author: rguenth
Date: Wed Jun 27 11:32:30 2012
New Revision: 189013

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189013
Log:
2012-06-27  Richard Guenther  <rguenther@suse.de>

    PR middle-end/53676
    * tree-chrec.c (chrec_convert_1): Represent truncation to
    a type with undefined overflow as truncation to an unsigned
    type converted to the type with undefined overflow.
    * tree-scalar-evolution.c (interpret_rhs_expr): For computing
    the scalar evolution of a truncated widened operation avoid
    looking at the non-existing evolution of the widened operation
    result.

    * gcc.dg/tree-ssa/scev-6.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/scev-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-chrec.c
    trunk/gcc/tree-scalar-evolution.c

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.3, 4.8.0
            Summary|[4.7/4.8 regression] empty  |[4.7 regression] empty loop
                   |loop is not always removed  |is not always removed now
                   |now                         |
      Known to fail|                            |4.7.1

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-27 11:33:12 UTC ---
Fixed for trunk.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #10 from Matt Hargett <matt at use dot net> 2012-06-27 18:26:55 UTC ---
Is there a fix targeted for 4.7.2? I can apply the patch and do some testing,
if that helps. Let me know what I can do, if anything, so we can make 4.7
deployable for us.

Thanks for the quick turnaround on the trunk commit!

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> 2012-06-28 08:26:12 UTC ---
On Wed, 27 Jun 2012, matt at use dot net wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676
> 
> --- Comment #10 from Matt Hargett <matt at use dot net> 2012-06-27 18:26:55 UTC ---
> Is there a fix targeted for 4.7.2? I can apply the patch and do some testing,
> if that helps. Let me know what I can do, if anything, so we can make 4.7
> deployable for us.

No, the fix isn't really suitable for backporting (we generally do not
fix but trivial missed-optimization regressions).

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #12 from Matt Hargett <matt at use dot net> 2012-08-21 21:40:11 UTC ---
I've been doing research into LLVM 3.1 and other GCC versions. LLVM 3.1
correctly eliminate the (near) empty loop, and their current trunk does not
regress like 4.7 has.

Is the trunk patch coupled to other changes that are too invasive for 4.7? I'm
confused and curious about what optimization regressions are serious enough to
warrant a back port, if any.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-08-22 08:46:54 UTC ---
(In reply to comment #12)
> I've been doing research into LLVM 3.1 and other GCC versions. LLVM 3.1
> correctly eliminate the (near) empty loop, and their current trunk does not
> regress like 4.7 has.
> 
> Is the trunk patch coupled to other changes that are too invasive for 4.7? I'm
> confused and curious about what optimization regressions are serious enough to
> warrant a back port, if any.

No, it's only the commit referenced in this PR.  No optimization regressions
warrant a backport as they always come with the risk of regressing something
worse than performance.  Trivial restoring of old behavior might be worth
backporting but the patch introduces a completely new non-trivial transform
into a core analysis engine that is shared among many passes.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |steven at gcc dot gnu.org

--- Comment #14 from Steven Bosscher <steven at gcc dot gnu.org> 2012-08-22 08:55:05 UTC ---
(In reply to comment #13)
> No, it's only the commit referenced in this PR.  No optimization regressions
> warrant a backport as they always come with the risk of regressing something
> worse than performance.  Trivial restoring of old behavior might be worth
> backporting but the patch introduces a completely new non-trivial transform
> into a core analysis engine that is shared among many passes.

FWIW, it seems to me that small patches, even non-trivial ones, should be
candidates for back-porting after they've been on the trunk or on a later
release branch for a reasonable period of time. E.g. after 3 months on the GCC
4.8 trunk and with no resulting bugs reported, this patch should be considered
for back-porting IMHO.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-08-22 09:17:10 UTC ---
(In reply to comment #14)
> (In reply to comment #13)
> > No, it's only the commit referenced in this PR.  No optimization regressions
> > warrant a backport as they always come with the risk of regressing something
> > worse than performance.  Trivial restoring of old behavior might be worth
> > backporting but the patch introduces a completely new non-trivial transform
> > into a core analysis engine that is shared among many passes.
> 
> FWIW, it seems to me that small patches, even non-trivial ones, should be
> candidates for back-porting after they've been on the trunk or on a later
> release branch for a reasonable period of time. E.g. after 3 months on the GCC
> 4.8 trunk and with no resulting bugs reported, this patch should be considered
> for back-porting IMHO.

Testing coverage during stage1 isn't very good IMHO, so we should wait at
least until we enter stage3.  But again, patch size isn't a good metric, it is
the coverage that counts - for this patch, changing SCEV analysis in a
non-trivial way is very intrusive.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[matt at use dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

--- Comment #16 from Matt Hargett <matt at use dot net> 2012-08-23 18:01:08 UTC ---
Back/forward-porting of the "trivial" restoration of the old behavior is
acceptable to me, as it would remove a major barrier to our adoption of 4.7.x.
That being said, if there's multi-platform testing I can do with a 'better' fix
based on trunk, I have SPARC and ARM qemu environments set up to regression
test on.

BTW, I realized that my initial analysis was wrong -- the int32_t and uint32_t
benchmarks are unaffected by this regression.

Let me know if there's an easy way to apply the restoration patch and I'll test
it locally on our amdfam10 and bdver2 hardware. If the testcase committed to
trunk would be applicable to a 4.7 fix as well, I can make a patch to integrate
that to ensure thie functionality doesn't regress again in future 4.7.x point
releases. I'm happy to do as much footwork as my expertise allows, just let me
know :)

Thanks!

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.2                       |4.7.3

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-20 10:14:49 UTC ---
GCC 4.7.2 has been released.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |laurent.alfonsi at st dot
                   |                            |com

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-10 08:11:36 UTC ---
*** Bug 56894 has been marked as a duplicate of this bug. ***

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.3                       |4.7.4

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-11 07:59:07 UTC ---
GCC 4.7.3 is being released, adjusting target milestone.

[Bug middle-end/53676] [4.7 regression] empty loop is not always removed now

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53676

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.7.4                       |4.8.0
      Known to fail|4.7.1                       |4.7.4

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for 4.8, not backporting.