[Bug target/56993] New: power gcc built 416.gamess generates wrong result

[Bug target/56993] New: power gcc built 416.gamess generates wrong result

[carrot at google dot com]

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

             Bug #: 56993
           Summary: power gcc built 416.gamess generates wrong result
    Classification: Unclassified
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: carrot@google.com
              Host: powerpc-linux-gnu
            Target: powerpc-linux-gnu
             Build: powerpc-linux-gnu


When I use the trunk gcc to run spec2006 416.gamess, I got the following error

$ runspec --config=test.cfg --tune=base --size=test --nofeedback --noreportable
game
runspec v6152 - Copyright 1999-2008 Standard Performance Evaluation Corporation
Using 'linux-ydl23-ppc' tools
Reading MANIFEST... 18357 files
Loading runspec modules................
Locating benchmarks...found 31 benchmarks in 6 benchsets.
Reading config file '/usr/local/google/carrot/spec2006/config/test.cfg'
Benchmarks selected: 416.gamess
Compiling Binaries
  Building 416.gamess base Linux64 default: (build_base_Linux64.0000)

Build successes: 416.gamess(base)

Setting Up Run Directories
  Setting up 416.gamess test base Linux64 default: created
(run_base_test_Linux64.0000)
Running Benchmarks
  Running (#1) 416.gamess test base Linux64 default

****************************************
Contents of exam29.err
****************************************
STOP IN ABRT

****************************************

*** Miscompare of exam29.out; for details see
   
/usr/local/google/carrot/spec2006/benchspec/CPU2006/416.gamess/run/run_base_test_Linux64.0000/exam29.out.mis
Invalid run; unable to continue.
If you wish to ignore errors please use '-I' or ignore_errors

The log for this run is in
/usr/local/google/carrot/spec2006/result/CPU2006.111.log
The debug log for this run is in
/usr/local/google/carrot/spec2006/result/CPU2006.111.log.debug

*
* Temporary files were NOT deleted; keeping temporaries such as
* /usr/local/google/carrot/spec2006/result/CPU2006.111.log.debug and
* /usr/local/google/carrot/spec2006/tmp/CPU2006.111
* (These may be large!)
*
runspec finished at Wed Apr 17 16:37:27 2013; 93 total seconds elapsed



My gcc is configured as

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/powerpc-linux-gnu/4.6/lto-wrapper
Target: powerpc-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.6.2-12'
--with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.6 --enable-shared --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6
--libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --enable-plugin --enable-objc-gc --enable-secureplt
--disable-softfloat --enable-targets=powerpc-linux,powerpc64-linux
--with-cpu=default32 --with-long-double-128 --enable-checking=release
--build=powerpc-linux-gnu --host=powerpc-linux-gnu --target=powerpc-linux-gnu
Thread model: posix
gcc version 4.6.2 (Debian 4.6.2-12)


GCC4.8 has the same error, but gcc4.7 is good.

[Bug target/56993] power gcc built 416.gamess generates wrong result

[carrot at google dot com]

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

--- Comment #1 from Carrot <carrot at google dot com> ---
I did some more experimentation on this benchmark.

O0/O1 generates correct result, but O2/Os generates wrong result. So the
problem should be in some optimization pass that is enabled in O2/Os while
disabled in O1.

[Bug target/56993] power gcc built 416.gamess generates wrong result

[mikpelinux at gmail dot com]

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

--- Comment #2 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Can you provide a reduced test case?

[Bug target/56993] power gcc built 416.gamess generates wrong result

[carrot at google dot com]

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

--- Comment #3 from Carrot <carrot at google dot com> ---
I don't have a reduced test case. But I have a reduced config file.

#######################################################
ext             = Linux64
backup_config   = 0
makeflags       = -j64

default=default=default=default:

# Set the path that the compiler is located at.
CC_PATH         = /usr/local/google/carrot/bin/trunkbin/bin

CC      = $(CC_PATH)/gcc
CXX     = $(CC_PATH)/g++
FC      = $(CC_PATH)/gfortran

OPTIMIZE   = -O2 -m64

CPORTABILITY = -DSPEC_CPU_LP64
CXXPORTABILITY = -DSPEC_CPU_LP64
FPPPORTABILITY = -DSPEC_CPU_LP64

[Bug target/56993] power gcc built 416.gamess generates wrong result

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #4 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I can see this as well; note that adding -fno-aggressive-loop-optimizations
helped.

[Bug target/56993] power gcc built 416.gamess generates wrong result

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Looking at 416.gamess, I see mess like:
$ cat /tmp/a.f90
  common /a/ i(10)
  i(:) = 1
  call foo (10)
end
$ cat /tmp/b.f90
subroutine foo (j)
  common /a/ i(1)
  l = 0
  do k = 1, j
    l = l + i(k)
  end do
  print *, l
end subroutine foo
$ gfortran -o /tmp/a /tmp/a.f90 /tmp/b.f90 -O2 -Wall; /tmp/a; gfortran -o
/tmp/b /tmp/a.f90 /tmp/b.f90 -O2 -Wall -fno-aggressive-loop-optimizations;
/tmp/b; gfortran -o /tmp/c /tmp/a.f90 /tmp/b.f90; /tmp/c
           1
          10
          10

Is that valid Fortran at all?  To the middle-end, the array (at the end of) the
COMMON block is represented as a structure containing a fixed bound array, so
the middle-end (rightfully) thinks that only X(1) is valid, but X(2) and higher
are undefined behavior.  If Fortran as language allows this, either everywhere,
or for compatibility in some mode, it would be better if such common blocks
were modeled similarly to C99 flexible array members with GNU extension of
initializing that; which is that the type of last RECORD_TYPE's FIELD_DECL
should have NULL max bound, and only the VAR_DECL would have size reflecting
the actual array size.  Of course, only the last array in the COMMON block
could be handled this way, if some (broken?) Fortran code still wants say
COMMON /XX/ X(1), B, C, D, Y(20)
and happily access X(2) as B, X(3) as C, X(4) as D, X(6) as Y(2) etc., then
I'm afraid -fno-aggressive-loop-optimizations is the only thing to do for such
code (and probably more than that).
>From the comments in unport.F, it looks like 416.gamess has commented out even
far uglier hacks, in particular using the COMMON /FPCOM / X(1) and then
when accessing it biasing index by some variable, which is initialized as a
difference between a heap region and the array in the common block.  That would
be even worse for the middle-end and C semantics, dunno what Fortran as
language ever allowed and what needs to be (optionally?) tolerated to get badly
written code working.

[Bug target/56993] power gcc built 416.gamess generates wrong result

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

--- Comment #6 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
  Some SPEC benchmarks contain questionable code.  It is true for SPEC2000 and
true for SPEC2006.  So the PR is not a surprise for me.

  Unfortunately, nobody from GCC community participates in SPEC org which
chooses the benchmarks.  Intel is a major player there (besides Sun in the
past) and they mostly care only about their compilers.

  Also in SPEC2006 they made very difficult to change the benchmark code (by
checking control sum of the code).

[Bug target/56993] power gcc built 416.gamess generates wrong result

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

ktkachov at gcc dot gnu.org changed:

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

--- Comment #7 from ktkachov at gcc dot gnu.org ---
> I'm afraid -fno-aggressive-loop-optimizations is the only thing to do for
> such code (and probably more than that).

Yeah, I use -fno-aggressive-loop-optimizations on gamess in my runs, I was
getting miscompares otherwise

[Bug target/56993] power gcc built 416.gamess generates wrong result

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
>   common /a/ i(10)
> subroutine foo (j)
>   common /a/ i(1)
> Is that valid Fortran at all?

No. Fortran 2008 (in 5.7.2.5) has: "Named common blocks of the same name shall
be of the same size in all scoping units of a program in which they appear"

That's not new as FORTRAN 77 (in 8.3.3) had: "Within an executable program, all
named common blocks that have the same name must be the same size".

The example has a named common block with name "A". Admittedly, that's not
really the issue for the wrong code here - as one could simply turn it into a
blank common: "but blocks of the same name shall be of the same size in at
blank common blocks may be of different sizes." [continuation of F2008 quote].


>   call foo (10)
> end
> subroutine foo (j)
>   common /a/ i(1)
...
>   do k = 1, j
>     l = l + i(k)
>
> Is that valid Fortran at all? 

In terms of the standard: No. Fortran 2008 explicitly states: "The value of a
subscript in an array element shall be within the bounds for its dimension."
(F2008, 6.5.3.)

For Fortran 77, I could only find something less explicit: "The first  element 
of  the  array has a subscript value of one; the second element has a subscript
value  of  two; the  last  element  has  a subscript value equal to the size of
the array." and "The size of an array is equal to the  product  of  the  sizes 
of  the dimensions  specified  by the array declarator for that array name."

Hence, I'd claim that anything but 1 is already invalid in Fortran 77. (Storage
associations in general allows some tricks, such as with using EQUIVALENCE (cf.
C/C++'s union) but this does not apply here.)


In Fortran 66/77, it was alledgedly not uncommon to write "dimension(1)" for
dummy/formal arguments - instead of using dimension(*) - and then to access
array elements beyond the first one. (Also invalid back then.) However, for
COMMON, that's new to me. (Not that I have much experience with pre-Fortran-90
code.)


> If Fortran as language allows this, either everywhere, or for compatibility
> in some mode, it would be better if such common blocks were modeled similarly
> to C99 flexible array members with GNU extension of initializing that;

Hmm, the big problem is where to draw a boundary and what to support. As such
things were never allowed by the standard, it is difficult to decide what to
support and using which flag. In real-world code, one can find tons of code
which was invalid but worked - often for years and with different compilers -
until it suddenly stopped doing so. It seems as if GCC/gfortran is in
particular successful in doing so (judging from bug reports) - which implies
that it seems to be better at exploiting optimization opportunities given by
the language.

A similar use to the one above would be:

integer A
integer B
COMMON /foo/ A(5), B(7)

and accessing A(8). That memory reference matches B(3) and is a valid memory
address. (Fortran's storage association rules guarantee this.) Still, it
exceeds "A" and is hence invalid according to the Fortran standard - and I
assume also the middle end does not like A(8).

I think for that memory access, the VLA trick wouldn't work. I try to avoid
such legacy extensions as they are ill-defined.


In any case, gfortran with -fcheck=bounds should reject this at run time - and
using a compile-time array index it should do so already at compile time. 


> From the comments in unport.F, it looks like 416.gamess has commented out
> even far uglier hacks, in particular using the COMMON /FPCOM / X(1) and then
> when accessing it biasing index by some variable, which is initialized as a
> difference between a heap region and the array in the common block.  That
> would be even worse for the middle-end and C semantics, dunno what Fortran
> as language ever allowed and what needs to be (optionally?) tolerated to get
> badly written code working.

Well, I think in may cases, the early versions of the language - like the
Fortran manuals from IBM - simply didn't state what would happen in that case.
But back in the old days, for many the standard matched what their compiler
did.

For instance a loop like "do i = 5, 3": How often is it executed? (Fortran 66
didn't permit it - but one can still write it.) Answer: With some vendors once,
with other vendors zero times. I think already Fortran 77 standardized this to
execute zero times. As the execute-once version was common, several compilers
(including Intel's current one, g77 and others) had a compile-time flag to
permit this.

(Actually, if I look at the code gfortran generates for a loop, it is written
such that once-trip loops could be simply enabled, even though no flag exists
to turn it on.)


(In reply to Vladimir Makarov from comment #6)
> Some SPEC benchmarks contain questionable code.  It is true for SPEC2000
> and true for SPEC2006.  So the PR is not a surprise for me.
> 
>  Unfortunately, nobody from GCC community participates in SPEC org

I think it would be useful if they could run the code before inclusion with all
checking turned on (gfortran's -fcheck=all -fsanitize=*, Intel's "-check all").
I think some of the issues would then be found before.

With a small test program, mimicing the code Jakub has posted, Intel's ifort
-CB gives: "Subscript #1 of the array I has value 2 which is greater than the
upper bound of 1"

[Bug target/56993] power gcc built 416.gamess generates wrong result

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2014-09-19
     Ever confirmed|0                           |1

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
This has already been discussed for gfortran in PR44882 and PR53086 and seems
related to PR53265.

[Bug target/56993] power gcc built 416.gamess generates wrong result

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56993

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 33517
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33517&action=edit
Here is my spec 2006 patch

I need this patch on x86.