[Bug middle-end/53073] New: [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[Bug middle-end/53073] New: [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[hjl.tools at gmail dot com]

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

             Bug #: 53073
           Summary: [4.8 Regression] 464.h264ref in SPEC CPU 2006
                    miscompiled
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com
                CC: areg.melikadamyan@gmail.com, rguenth@gcc.gnu.org


On Linux/x86-64, revision 186592:

http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00543.html

compiles 464.h264ref in SPEC CPU 2006 into infinite loop.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-23 09:20:37 UTC ---
Well, I'm sure it is h264refs fault.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-23 11:29:58 UTC ---
1016    int
1017    SATD (int* diff, int use_hadamard)
1018    {
1019      int k, satd = 0, m[16], dd, d[16];
...
1092        /*===== sum up =====*/
1093        for (dd=d[k=0]; k<16; dd=d[++k])
1094        {
1095          satd += (dd < 0 ? -dd : dd);
1096        }

accesses d[16].

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-23 11:44:11 UTC ---
Fixed by doing

    /*===== sum up =====*/
    for (k=0; k < 16; k++)
    {
      dd = d[k];
      satd += (dd < 0 ? -dd : dd);
    }

instead.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[venkataramanan.kumar at amd dot com]

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

Venkataramanan <venkataramanan.kumar at amd dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |venkataramanan.kumar at amd
                   |                            |dot com

--- Comment #4 from Venkataramanan <venkataramanan.kumar at amd dot com> 2012-08-21 06:27:29 UTC ---
is there are src.alts available for this benchmark ?

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |josh.m.conner at gmail dot
                   |                            |com

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-11-08 10:42:25 UTC ---
*** Bug 55216 has been marked as a duplicate of this bug. ***

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[mpaton at swbell dot net]

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

Michael Paton <mpaton at swbell dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpaton at swbell dot net

--- Comment #6 from Michael Paton <mpaton at swbell dot net> 2013-01-15 17:02:31 UTC ---
I would like to question the resolution status of this bug.

Perhaps I will be educated on the process used here, but I fail to see how
supplying different source code fixes this bug. Making it go away, IMO is not
the same as fixing it.

The original code is likely not what the original author intended, and the
alternate code supplied by Richard Biener is by some measures "better".

However the original code is NOT invalid C code. It certainly reads memory past
the end of an array, and so assigns an undetermined value to the variable dd.
However this action is

1 Still valid C code
2 does not affect the control flow of the for loop in the code as written

Furthermore, the "endless loop" is in fact a jump to the current instruction,
with no sign of code for the accesses of arrays d and m.

While it is inconvenient for the optimizer to discover source code like this,
the optimizer is free to either not optimize, or complain via a diagnostic, or
in the extreme to refuse to compile. I do not believe it is free to silently
emit bad code in response to inconvenient, valid C ode.

I'd be happy to be educated on why the emitted code could be valid; currently I
do not see how it could be.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-15 17:56:05 UTC ---
You're wrong, the code in #c2 is not valid C.
ISO C99 6.5.2.1 says that d[++k] is equivalent to:
  (*((d)+(++k)))
and ++k in the last iteration is 16, so it is
  (*(d+16))
and then 6.5.6/8 (last sentence) applies:
"If the result points one past the last element of the array object, it
shall not be used as the operand of a unary * operator that is evaluated."
So, if you ever enter this loop, you'll invoke undefined behavior and anything
can happen.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[mpaton at swbell dot net]

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

--- Comment #8 from Michael Paton <mpaton at swbell dot net> 2013-01-15 20:01:04 UTC ---
Hopefully it's correct to reply to the mailing list rather than to bugzilla.
I've tried to find definitive guidance on that and failed.

Thanks for your informative reply asserting C99 non compliance. My assertion on
validity was C89 based, but let's use yours and move on.

I accept that the source code in #c2 is invalid C99 code, and that undefined
behavior may be the result.

However, I will still argue that undefined behavior should be a diagnostic, or
a
lack of an optimizing transformation. If the compiler is to be silent on
discovering this invalid C code, I do not believe it should go on to produce
code that runs until it starts executing an infinite loop.

While that may be allowed by ISO C99, I have a hard time believing that it is
useful to the users of gcc, or indeed intended by its contributors. This
invalid
code compiles and executes with -O0, yet generates bad assembly code ending in
an infinite loop with -O2. I think this is indicative of a bug in gcc 4.8.0. As
to whether such a bug might influence valid code, I cannot say.

Michael Paton

On 1/15/2013 11:56 AM, jakub at gcc dot gnu.org wrote:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53073
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-15 17:56:05 UTC ---
> You're wrong, the code in #c2 is not valid C.
> ISO C99 6.5.2.1 says that d[++k] is equivalent to:
>   (*((d)+(++k)))
> and ++k in the last iteration is 16, so it is
>   (*(d+16))
> and then 6.5.6/8 (last sentence) applies:
> "If the result points one past the last element of the array object, it
> shall not be used as the operand of a unary * operator that is evaluated."
> So, if you ever enter this loop, you'll invoke undefined behavior and anything
> can happen.
>

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[sje at gcc dot gnu.org]

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

Steve Ellcey <sje at gcc dot gnu.org> changed:

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

--- Comment #9 from Steve Ellcey <sje at gcc dot gnu.org> 2013-01-28 18:50:42 UTC ---
FYI: I reported this issue to SPEC to make sure they were aware of it.
their reply is at:

http://www.spec.org/cpu2006/Docs/faq.html#Run.05

They do not intend to offer alternative sources.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[rguenther at suse dot de]

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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> 2013-01-29 09:10:25 UTC ---
On Mon, 28 Jan 2013, sje at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53073
> 
> Steve Ellcey <sje at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |sje at gcc dot gnu.org
> 
> --- Comment #9 from Steve Ellcey <sje at gcc dot gnu.org> 2013-01-28 18:50:42 UTC ---
> FYI: I reported this issue to SPEC to make sure they were aware of it.
> their reply is at:
> 
> http://www.spec.org/cpu2006/Docs/faq.html#Run.05
> 
> They do not intend to offer alternative sources.

So be it.  At a last resort we could add a switch that disables
just number-of-iteration computations based on undefined behavior.
Note that all previous releases of GCC have the same behavior - the
knowledge is just not used.  Thus such switch would pessimize
code further than reverting to previous behavior - nevertheless
such switch would be consistent with existing switches like
-fno-strict-aliasing or -fno-strict-overflow.

Richard.

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-31 09:01:02 UTC ---
Author: rguenth
Date: Thu Jan 31 09:00:54 2013
New Revision: 195610

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195610
Log:
2013-01-31  Richard Biener  <rguenther@suse.de>

    PR middle-end/53073
    * common.opt (faggressive-loop-optimizations): New flag,
    enabled by default.
    * doc/invoke.texi (faggressive-loop-optimizations): Document.
    * tree-ssa-loop-niter.c (estimate_numbers_of_iterations_loop): Guard
    infer_loop_bounds_from_undefined by it.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/common.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/tree-ssa-loop-niter.c

[Bug middle-end/53073] [4.8 Regression] 464.h264ref in SPEC CPU 2006 miscompiled

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-31 09:28:28 UTC ---
FYI, regarding validity in C89, the wording there is similar to C99.
ISO C89, 6.3.1.2 says that d[++k] is equivalent to:
  (*((d)+(++k)))
and ++k in the last iteration is 16, so it is
  (*(d+16))
and then 6.3.6 part applies:
"Unless both the pointer operand and the result point to elements of the
same array object, or the pointer operand points one past the last element  
of an array object and the result points to an element of the same array
object, the behavior is undefined if the result is used as an operand of the 
unary * operator."
Here the pointer operand points to the first element of array (&d[0]) and
the result points one past the last element of the array object, so again, the
behavior is undefined.