public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/58416] New: Incorrect x87-based union copying code
@ 2013-09-13 19:56 stichnot at google dot com
  2013-09-13 20:23 ` [Bug target/58416] " hjl.tools at gmail dot com
                   ` (27 more replies)
  0 siblings, 28 replies; 29+ messages in thread
From: stichnot at google dot com @ 2013-09-13 19:56 UTC (permalink / raw)
  To: gcc-bugs

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 14191 bytes --]

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

            Bug ID: 58416
           Summary: Incorrect x87-based union copying code
           Product: gcc
           Version: 4.6.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: stichnot at google dot com

Created attachment 30819
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30819&action=edit
.ii file generated from nantest1.cpp

The program below produces incorrect code on x86-32 with gcc 4.6.3.  (It was
also found to fail in 4.7 but succeed in 4.8.)

The problem happens when:
1. The union is initialized via its 64-bit double field
2. The union is then updated via its 64-bit integer field
3. The union is copied to another variable

When this happens, the 64-bit copy in step 3 is done via the x87 fldl/fstpl
instructions.  If the 64-bit integer value in step 2 happens to be an sNaN
pattern, the fldl instruction transforms it to a qNaN pattern and the fstpl
instruction writes the wrong value.

If step 1 is omitted, the x87 instructions are not used and the code is
correct.  This suggests that the optimization of using x87 for 64-bit transfers
is enabled when a use of the double field is found, whereas it should probably
only happen if a *live* use of the double field is found.


// Compile with g++ -O3 -m32
// -O[123] all exhibit the problem.

#include <assert.h>
#include <stdlib.h>
#include <stdio.h>

union MyClass {
  MyClass() : DoubleVal(0.0) {} // this ctor causes failure
  //MyClass() : IntVal(0) {} // this ctor works correctly
  int64_t IntVal;
  double DoubleVal;
};

__attribute__((noinline)) void create(MyClass &dest, int64_t Val) {
  MyClass MCOp;
  MCOp.IntVal = Val;
  dest = MCOp;
}

int main(int argc, char **argv) {
  MyClass copy;

  int64_t magic = 0xfff0000000000001; // happens to be sNaN
  create(copy, magic); // copy is changed to qNaN

  printf("magic=%llx\ncopy= %llx\n", magic, copy.IntVal);
  assert(magic == copy.IntVal);

  return 0;
}


$ g++ -v -save-temps -Wall -Wextra -fno-strict-aliasing -fwrapv -O3 -m32
nantest1.cpp 
Using built-in specs.
COLLECT_GCC=/usr/bin/g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.6.3-1ubuntu5' --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 --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object
--enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686
--with-tune=generic --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra' '-fno-strict-aliasing'
'-fwrapv' '-O3' '-m32' '-shared-libgcc' '-mtune=generic' '-march=i686'
 /usr/lib/gcc/x86_64-linux-gnu/4.6/cc1plus -E -quiet -v -imultilib 32
-imultiarch i386-linux-gnu -D_GNU_SOURCE nantest1.cpp -m32 -mtune=generic
-march=i686 -Wall -Wextra -fno-strict-aliasing -fwrapv -O3 -fpch-preprocess
-fstack-protector -o nantest1.ii
ignoring nonexistent directory "/usr/local/include/i386-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../x86_64-linux-gnu/include"
ignoring nonexistent directory "/usr/include/i386-linux-gnu"
#include "..." search starts here:
#include <...> search starts here:
 /usr/include/c++/4.6
 /usr/include/c++/4.6/x86_64-linux-gnu/32
 /usr/include/c++/4.6/backward
 /usr/lib/gcc/x86_64-linux-gnu/4.6/include
 /usr/local/include
 /usr/lib/gcc/x86_64-linux-gnu/4.6/include-fixed
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra' '-fno-strict-aliasing'
'-fwrapv' '-O3' '-m32' '-shared-libgcc' '-mtune=generic' '-march=i686'
 /usr/lib/gcc/x86_64-linux-gnu/4.6/cc1plus -fpreprocessed nantest1.ii -quiet
-dumpbase nantest1.cpp -m32 -mtune=generic -march=i686 -auxbase nantest1 -O3
-Wall -Wextra -version -fno-strict-aliasing -fwrapv -fstack-protector -o
nantest1.s
GNU C++ (Ubuntu/Linaro 4.6.3-1ubuntu5) version 4.6.3 (x86_64-linux-gnu)
    compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++ (Ubuntu/Linaro 4.6.3-1ubuntu5) version 4.6.3 (x86_64-linux-gnu)
    compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 65b5171ac1bd7b3f07dbea6bdb24be3d
nantest1.cpp:21:5: warning: unused parameter ‘argc’ [-Wunused-parameter]
nantest1.cpp:21:5: warning: unused parameter ‘argv’ [-Wunused-parameter]
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra' '-fno-strict-aliasing'
'-fwrapv' '-O3' '-m32' '-shared-libgcc' '-mtune=generic' '-march=i686'
 as --32 -o nantest1.o nantest1.s
COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/4.6/:/usr/lib/gcc/x86_64-linux-gnu/4.6/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/4.6/:/usr/lib/gcc/x86_64-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/4.6/32/:/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../i386-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../lib32/:/lib/i386-linux-gnu/:/lib/../lib32/:/usr/lib/i386-linux-gnu/:/usr/lib/../lib32/:/usr/lib/gcc/x86_64-linux-gnu/4.6/:/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../i386-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../:/lib/i386-linux-gnu/:/lib/:/usr/lib/i386-linux-gnu/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra' '-fno-strict-aliasing'
'-fwrapv' '-O3' '-m32' '-shared-libgcc' '-mtune=generic' '-march=i686'
 /usr/lib/gcc/x86_64-linux-gnu/4.6/collect2 --sysroot=/ --build-id
--no-add-needed --as-needed --eh-frame-hdr -m elf_i386 --hash-style=gnu
-dynamic-linker /lib/ld-linux.so.2 -z relro
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../lib32/crt1.o
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../lib32/crti.o
/usr/lib/gcc/x86_64-linux-gnu/4.6/32/crtbegin.o
-L/usr/lib/gcc/x86_64-linux-gnu/4.6/32
-L/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../i386-linux-gnu
-L/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../lib32 -L/lib/i386-linux-gnu
-L/lib/../lib32 -L/usr/lib/i386-linux-gnu -L/usr/lib/../lib32
-L/usr/lib/gcc/x86_64-linux-gnu/4.6
-L/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../i386-linux-gnu
-L/usr/lib/gcc/x86_64-linux-gnu/4.6/../../.. -L/lib/i386-linux-gnu
-L/usr/lib/i386-linux-gnu nantest1.o -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s
-lgcc /usr/lib/gcc/x86_64-linux-gnu/4.6/32/crtend.o
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../lib32/crtn.o
>From gcc-bugs-return-429782-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Sep 13 20:17:37 2013
Return-Path: <gcc-bugs-return-429782-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 1636 invoked by alias); 13 Sep 2013 20:17:37 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 1599 invoked by uid 48); 13 Sep 2013 20:17:33 -0000
From: "jorge.aparicio.r at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/57657] [regression from 4.7] Reports incorrect cache sizes on corei7
Date: Fri, 13 Sep 2013 20:17:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 4.8.1
X-Bugzilla-Keywords:
X-Bugzilla-Severity: minor
X-Bugzilla-Who: jorge.aparicio.r at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: cc
Message-ID: <bug-57657-4-ObmpqyLWEC@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-57657-4@http.gcc.gnu.org/bugzilla/>
References: <bug-57657-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-09/txt/msg01022.txt.bz2
Content-length: 5111

http://gcc.gnu.org/bugzilla/show_bug.cgi?idW657

Jorge Aparicio <jorge.aparicio.r at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jorge.aparicio.r at gmail dot com

--- Comment #1 from Jorge Aparicio <jorge.aparicio.r at gmail dot com> ---
I can confirm this gcc-4.8.1 regression on three different Intel CPUs (core i3
2nd gen, core i5 1rst gen and core i7 4th gen), but the regression actually
comes from gcc-4.8.0

CONCLUSION:
gcc-4.8.0 reports the correct cache sizes when -march=native is used
gcc-4.8.1 always reports 1) l1 cache size = 0, 2) l1 line cache size = 0 and 3)
l2 cache size = 256; when -march=native is used

OBSERVATIONS:
#### Processor 1 ####
$ lscpu | grep 'name\|cache'
Model name:            Intel(R) Core(TM) i7-4702MQ CPU @ 2.20GHz
L1d cache:             32K
L1i cache:             32K
L2 cache:              256K
L3 cache:              6144K

$ gcc 4.8.0 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size2 --param l1-cache-line-sized --param
l2-cache-sizea44 ...

$ gcc 4.8.1 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size=0 --param l1-cache-line-size=0 --param
l2-cache-size%6 ...

#### Processor 2 ####
$ lscpu | grep 'name\|cache'
Model name:            Intel(R) Core(TM) i5 CPU         750  @ 2.67GHz
L1d cache:             32K
L1i cache:             32K
L2 cache:              256K
L3 cache:              8192K

$ gcc-4.8.0 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size2 --param l1-cache-line-sized --param
l2-cache-size92 ...

$ gcc 4.8.1 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size=0 --param l1-cache-line-size=0 --param
l2-cache-size%6 ...

#### Processor 3 ####
$ lscpu | grep 'name\|cache'
Model name:            Intel(R) Core(TM) i3-2330M CPU @ 2.20GHz
L1d cache:             32K
L1i cache:             32K
L2 cache:              256K
L3 cache:              3072K

$ gcc 4.8.0 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size2 --param l1-cache-line-sized --param
l2-cache-size072 ...

$ gcc 4.8.1 -march=native -E -v - </dev/null 2>&1 | grep cc1
... --param l1-cache-size=0 --param l1-cache-line-size=0 --param
l2-cache-size%6 ...

#### GCC version information ####
$ gcc-4.8.1 -v
Using built-in specs.
COLLECT_GCC=gcc-4.8.1
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/4.8.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-4.8.1/configure --disable-libssp --disable-multilib
--enable-version-specific-runtime-libs --enable-libmudflap --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/4.8.1
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.1/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.1
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.1/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.1/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.1/include/g++-v4.8
--enable-__cxa_atexit --enable-clocale=gnu --host=x86_64-pc-linux-gnu
--build=x86_64-pc-linux-gnu --disable-ppl --disable-cloog --with-system-zlib
--enable-obsolete --disable-werror --enable-secureplt --disable-lto
--with-bugurl=http://bugs.funtoo.org --with-pkgversion='Funtoo 4.8.1'
--with-mpfr-include=/var/tmp/portage/sys-devel/gcc-4.8.1/work/gcc-4.8.1/mpfr/src
--with-mpfr-lib=/var/tmp/portage/sys-devel/gcc-4.8.1/work/objdir/mpfr/src/.libs
--enable-libstdcxx-time --enable-libgomp --enable-languages=c,c++,fortran
--disable-libgcj --disable-esp
Thread model: posix
gcc version 4.8.1 (Funtoo 4.8.1)

$ gcc-4.8.0 -v
Using built-in specs.
COLLECT_GCC=gcc-4.8.0
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/4.8.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-4.8.0/work/gcc-4.8.0/configure
--prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/4.8.0
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.0
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.0/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/4.8.0/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/include/g++-v4
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --disable-altivec
--disable-fixed-point --with-ppl --with-cloog --disable-isl-version-check
--with-cloog --enable-lto --disable-nls --with-system-zlib --enable-obsolete
--disable-werror --enable-secureplt --disable-multilib --with-multilib-list=m64
--disable-libmudflap --disable-libssp --enable-libgomp
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/4.8.0/python
--enable-checking=release --disable-libgcj --enable-libstdcxx-time
--enable-languages=c,c++,fortran --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-targets=all
--with-bugurl=http://bugs.gentoo.org/ --with-pkgversion='Gentoo 4.8.0 p1.1,
pie-0.5.5'
Thread model: posix
gcc version 4.8.0 (Gentoo 4.8.0 p1.1, pie-0.5.5)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
@ 2013-09-13 20:23 ` hjl.tools at gmail dot com
  2013-09-16 10:28 ` rguenth at gcc dot gnu.org
                   ` (26 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: hjl.tools at gmail dot com @ 2013-09-13 20:23 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> ---
This may be related to PR57484.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
  2013-09-13 20:23 ` [Bug target/58416] " hjl.tools at gmail dot com
@ 2013-09-16 10:28 ` rguenth at gcc dot gnu.org
  2013-09-16 13:21 ` ubizjak at gmail dot com
                   ` (25 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2013-09-16 10:28 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |i?86-*-*

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
It is SRA that transforms

  MCOp.DoubleVal = 0.0;
  MCOp.IntVal = Val_3(D);
  *dest_5(D) = MCOp;

into

  MCOp$DoubleVal_7 = 0.0;
  _8 = VIEW_CONVERT_EXPR<double>(Val_3(D));
  MCOp$DoubleVal_4 = _8;
  MEM[(union MyClass *)dest_5(D)] = MCOp$DoubleVal_4;

simplifying into

  _6 = VIEW_CONVERT_EXPR<double>(Val_3(D));
  MEM[(union MyClass *)dest_4(D)] = _6;

and

(insn 7 6 0 (set (mem:DF (reg/v/f:SI 60 [ dest ]) [0 MEM[(union MyClass
*)dest_4(D)]+0 S8 A32])
        (subreg:DF (reg/v:DI 61 [ Val ]) 0)) t.C:15 -1
     (nil))

causes a reload:

(insn 7 11 12 2 (set (reg:DF 8 st [62])
        (mem/c:DF (plus:SI (reg/f:SI 7 sp)
                (const_int 8 [0x8])) [0 Val+0 S8 A32])) t.C:15 134
{*movdf_internal}
     (nil))
(insn 12 7 0 2 (set (mem:DF (reg/v/f:SI 0 ax [orig:60 dest ] [60]) [0
MEM[(union MyClass *)dest_4(D)]+0 S8 A32])
        (reg:DF 8 st [62])) t.C:15 134 {*movdf_internal}
     (expr_list:REG_DEAD (reg:DF 8 st [62])
        (expr_list:REG_DEAD (reg/v/f:SI 0 ax [orig:60 dest ] [60])
            (nil))))

where *movdf_internal simply doesn't properly preserve sNaN.

It looks wrong for DFmode move instructions to not preserve a IEEE defined
flag.  I suppose it would be also wrong to not preserve the exact bit pattern
(think of canonicalizing NaNs).


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
  2013-09-13 20:23 ` [Bug target/58416] " hjl.tools at gmail dot com
  2013-09-16 10:28 ` rguenth at gcc dot gnu.org
@ 2013-09-16 13:21 ` ubizjak at gmail dot com
  2013-09-16 15:49 ` joseph at codesourcery dot com
                   ` (24 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: ubizjak at gmail dot com @ 2013-09-16 13:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Richard Biener from comment #2)

> where *movdf_internal simply doesn't properly preserve sNaN.
> 
> It looks wrong for DFmode move instructions to not preserve a IEEE defined
> flag.  I suppose it would be also wrong to not preserve the exact bit pattern
> (think of canonicalizing NaNs).

As said in PR57484, comment 11:

On an x86 target using the legacy x87 instructions and the 80-bit registers, a
load of a 64-bit or 32-bit value in memory into the 80-bit registers counts as
a format conversion and an signaling NaN input will turn into a quiet NaN in
the register format.
>From gcc-bugs-return-429905-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Sep 16 13:22:56 2013
Return-Path: <gcc-bugs-return-429905-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 14684 invoked by alias); 16 Sep 2013 13:22:56 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 14662 invoked by uid 48); 16 Sep 2013 13:22:52 -0000
From: "valeryweber at hotmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/58434] New: no automatic deallocation with trunk
Date: Mon, 16 Sep 2013 13:22:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: valeryweber at hotmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter
Message-ID: <bug-58434-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-09/txt/msg01145.txt.bz2
Content-length: 2364

http://gcc.gnu.org/bugzilla/show_bug.cgi?idX434

            Bug ID: 58434
           Summary: no automatic deallocation with trunk
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: valeryweber at hotmail dot com

Dear All
I noticed a changed of behavior between 4.8.1 and trunk.
While the following code produde no memory leaks with 4.8.1 it does with
the trunk. Is that a bug?
v


cat tmp.f90
module mod
  type t
     integer,allocatable,dimension(:)::i
  end type t
end module mod
program main
  use mod
  type(t) :: a
  allocate(a%i(10000))
end program main
gfortran-trunk -g tmp.f90
valgrind ./a.out
=\x13501== Memcheck, a memory error detector
=\x13501== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
=\x13501== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
=\x13501== Command: ./a.out
=\x13501==\x13501==\x13501== HEAP SUMMARY:
=\x13501==     in use at exit: 40,000 bytes in 1 blocks
=\x13501==   total heap usage: 18 allocs, 17 frees, 43,688 bytes allocated
=\x13501==\x13501== LEAK SUMMARY:
=\x13501==    definitely lost: 0 bytes in 0 blocks
=\x13501==    indirectly lost: 0 bytes in 0 blocks
=\x13501==      possibly lost: 0 bytes in 0 blocks
=\x13501==    still reachable: 40,000 bytes in 1 blocks
=\x13501==         suppressed: 0 bytes in 0 blocks
=\x13501== Rerun with --leak-check=full to see details of leaked memory
=\x13501==\x13501== For counts of detected and suppressed errors, rerun with: -v
=\x13501== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
gfortran-4.8.1 -g tmp.f90
valgrind ./a.out
=\x13508== Memcheck, a memory error detector
=\x13508== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
=\x13508== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
=\x13508== Command: ./a.out
=\x13508==\x13508==\x13508== HEAP SUMMARY:
=\x13508==     in use at exit: 0 bytes in 0 blocks
=\x13508==   total heap usage: 18 allocs, 18 frees, 43,664 bytes allocated
=\x13508==\x13508== All heap blocks were freed -- no leaks are possible
=\x13508==\x13508== For counts of detected and suppressed errors, rerun with: -v
=\x13508== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (2 preceding siblings ...)
  2013-09-16 13:21 ` ubizjak at gmail dot com
@ 2013-09-16 15:49 ` joseph at codesourcery dot com
  2024-02-20 10:05 ` rguenth at gcc dot gnu.org
                   ` (23 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: joseph at codesourcery dot com @ 2013-09-16 15:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Mon, 16 Sep 2013, rguenth at gcc dot gnu.org wrote:

> It looks wrong for DFmode move instructions to not preserve a IEEE defined
> flag.  I suppose it would be also wrong to not preserve the exact bit pattern
> (think of canonicalizing NaNs).

I think of the move problem for signaling NaNs as being bug 56831 - except 
that as expressed there it's only a bug for -fsignaling-nans, whereas the 
present bug is clearly a bug with or without -fsignaling-nans (the program 
makes no explicit use of signaling NaNs at all).

In the absence of -fexcess-precision=standard, I suppose the move patterns 
do need to represent how to move DFmove values into x87 registers from 
memory (despite the lack of proper bit-pattern-preserving loads on the 
processor), but any move instruction that extends SFmode or DFmode 
implicitly to XFmode should be avoided except where actually needed for 
floating-point arithmetic to be carried out or because the ABI requires 
the value in an x87 register.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (3 preceding siblings ...)
  2013-09-16 15:49 ` joseph at codesourcery dot com
@ 2024-02-20 10:05 ` rguenth at gcc dot gnu.org
  2024-02-20 10:07 ` rguenth at gcc dot gnu.org
                   ` (22 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-02-20 10:05 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2024-02-20
                 CC|                            |jamborm at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Reconfirmed with the comment#5 testcase.  ESRA does

 int main ()
 {
+  long double x$d;
   unsigned char * p;
   union u y;
   union u x;
@@ -25,9 +20,9 @@
   int _2;

   <bb 2> :
-  x.d = 0.0;
-  x.s.s = "xxxxxxxxxxxxxxxx";
-  y = x;
+  x$d_3 = 0.0;
+  x$d_16 = MEM[(char[16] *)"xxxxxxxxxxxxxxxx"];
+  MEM[(union u *)&y] = x$d_16;

and the value re-interpretation goes off.  Martin - we may have dups of this
but SRA shouldn't do this (it possibly gets confused by the x.d store)?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (4 preceding siblings ...)
  2024-02-20 10:05 ` rguenth at gcc dot gnu.org
@ 2024-02-20 10:07 ` rguenth at gcc dot gnu.org
  2024-02-20 10:12 ` pinskia at gcc dot gnu.org
                   ` (21 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-02-20 10:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Access trees for x (UID: 2479):
access { base = (2479)'x', offset = 0, size = 128, expr = x.d, type = long
double, reverse = 0, grp_read = 1, grp_write = 1, grp_assignment_read = 1,
grp_assignment_write = 1, grp_scalar_read = 0, grp_scalar_write = 1,
grp_total_scalarization = 1, grp_hint = 0, grp_covered = 1,
grp_unscalarizable_region = 0, grp_unscalarized_data = 0, grp_same_access_path
= 0, grp_partial_lhs = 0, grp_to_be_replaced = 1, grp_to_be_debug_replaced = 0}

so it's odd to not see x.s.s access here.  I guess we merge them and somehow
the FP type "prevails" over the array type?  IMO when merging "bad" types
we have to pun to a bitwise type.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (5 preceding siblings ...)
  2024-02-20 10:07 ` rguenth at gcc dot gnu.org
@ 2024-02-20 10:12 ` pinskia at gcc dot gnu.org
  2024-07-18 14:16 ` akrl at gcc dot gnu.org
                   ` (20 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-02-20 10:12 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6);
> 
> and the value re-interpretation goes off.  Martin - we may have dups of this
> but SRA shouldn't do this (it possibly gets confused by the x.d store)?


Yes I think pr 93271 is a dup of this.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (6 preceding siblings ...)
  2024-02-20 10:12 ` pinskia at gcc dot gnu.org
@ 2024-07-18 14:16 ` akrl at gcc dot gnu.org
  2024-07-19 12:34 ` rguenth at gcc dot gnu.org
                   ` (19 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: akrl at gcc dot gnu.org @ 2024-07-18 14:16 UTC (permalink / raw)
  To: gcc-bugs

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

akrl at gcc dot gnu.org changed:

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

--- Comment #9 from akrl at gcc dot gnu.org ---
Hello,

this is apparently affecting Emacs build as well [1].
I there a suggested way we could work around this bug? Maybe a flag to prevent
the optimization which we could use on affected systems?

Thanks

  Andrea

[1] <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72145>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (7 preceding siblings ...)
  2024-07-18 14:16 ` akrl at gcc dot gnu.org
@ 2024-07-19 12:34 ` rguenth at gcc dot gnu.org
  2024-07-20 14:30 ` eggert at cs dot ucla.edu
                   ` (18 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-19 12:34 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to akrl from comment #9)
> Hello,
> 
> this is apparently affecting Emacs build as well [1].
> I there a suggested way we could work around this bug? Maybe a flag to
> prevent the optimization which we could use on affected systems?
> 
> Thanks
> 
>   Andrea
> 
> [1] <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72145>

The remaining issue is analyzed to be caused by SRA so you can check whether
-fno-tree-sra fixes it for you.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (8 preceding siblings ...)
  2024-07-19 12:34 ` rguenth at gcc dot gnu.org
@ 2024-07-20 14:30 ` eggert at cs dot ucla.edu
  2024-07-21  8:58 ` rguenth at gcc dot gnu.org
                   ` (17 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-07-20 14:30 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Paul Eggert <eggert at cs dot ucla.edu> ---
(In reply to Richard Biener from comment #10)
> The remaining issue is analyzed to be caused by SRA so you can check whether
> -fno-tree-sra fixes it for you.

Thanks, it does, and I modified the Emacs 'configure' script here:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9f4fc6608212191e1a9e07bf89f38ba9e4ea786c

so that, when using GCC 4 or later on x86, Emacs 'configure' appends to CFLAGS
either the option -mfpmath=sse (if the target is known to support SSE2, e.g.,
because -msse2 is also given) or the option -fno-tree-sra (if not).

It strikes me that GCC could do this itself, as a crude workaround for all
programs. That is, when compiling for x86 and -mfpmath=sse is not in effect,
GCC could automatically disable tree-sra for the compilation unit. Or it could
be more selective and disable tree-sra only if the compilation unit defines a
union that could cause the trouble.

Although such a patch would hurt the performance of the generated code, that's
better than generating wrong code that works only 99.9% of the time.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (9 preceding siblings ...)
  2024-07-20 14:30 ` eggert at cs dot ucla.edu
@ 2024-07-21  8:58 ` rguenth at gcc dot gnu.org
  2024-07-21  8:58 ` rguenth at gcc dot gnu.org
                   ` (16 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-21  8:58 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
I have two variants of a fix, one that for the testcase in comment#5 avoids
scalarization on i?86-*-* and one that scalarizes into unsigned:96 which
exposes that we fail to constant fold MEM<unsigned:96>["xxxxxxxxxxxxxxxx"];

On x86-64 with sizeof(long double) == 16 (instead of == 12 with -m32) we
scalarize with uint128_t.

I'm going to test the variant that avoids creating an unsigned:96 integer type
since I think that's safer.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (10 preceding siblings ...)
  2024-07-21  8:58 ` rguenth at gcc dot gnu.org
@ 2024-07-21  8:58 ` rguenth at gcc dot gnu.org
  2024-07-21 10:21 ` rguenth at gcc dot gnu.org
                   ` (15 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-21  8:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 58718
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58718&action=edit
patch I am testing

Paul - can you test if this patch resolves the emacs issue?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (11 preceding siblings ...)
  2024-07-21  8:58 ` rguenth at gcc dot gnu.org
@ 2024-07-21 10:21 ` rguenth at gcc dot gnu.org
  2024-07-22 21:09 ` jamborm at gcc dot gnu.org
                   ` (14 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-21 10:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #13)
> Created attachment 58718 [details]
> patch I am testing
> 
> Paul - can you test if this patch resolves the emacs issue?

Using root->grp_same_access_path doesn't seem to be a perfect fit.  For
gcc.dg/tree-ssa/sra-6.c we're now doing

Changing the type of a replacement for b offset: 0, size: 64  to an integer.

But maybe that's OK or we need a different flag to tell whether the access
paths are the same or just the tail of the access path, so 's' and 's.a'
would be OK for a root 's.a.f', allowing aggregate (sub-)copies but that
would assume we copy field-wise which we don't do - we RTL expand to
(inlined) memcpy which is also not semantically the same for x87 float
fields (but would it have to be?)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (12 preceding siblings ...)
  2024-07-21 10:21 ` rguenth at gcc dot gnu.org
@ 2024-07-22 21:09 ` jamborm at gcc dot gnu.org
  2024-07-23  1:47 ` eggert at cs dot ucla.edu
                   ` (13 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: jamborm at gcc dot gnu.org @ 2024-07-22 21:09 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 58724
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58724&action=edit
simple (wip) fix

I'm wondering whether just simply something like this would not be enough.  I
have looked at total scalarization and we will not replace a type found in the
IL with another one there. Similarly, only propagation through assignments
fiddles with existing types (when they are not aggregate) only when propagating
from RHS to LHS and not the other way round.

If we want to be more aggressive, we can add a flag when the new predicate
fails but there is a good bitwise_type_for_mode and then when the flag is set,
use that type instead in analyze_access_subtree.

Note that so far I have only tested the attached patch with
  make -k check-gcc RUNTESTFLAGS="tree-ssa.exp=*sra*.c"
  make -k check-g++ RUNTESTFLAGS="dg.exp=*sra*.c"
  make -k check-gcc RUNTESTFLAGS="dg.exp=*sra*.c"

I'll have a look at full test results tomorrow morning.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (13 preceding siblings ...)
  2024-07-22 21:09 ` jamborm at gcc dot gnu.org
@ 2024-07-23  1:47 ` eggert at cs dot ucla.edu
  2024-07-23  6:23 ` rguenth at gcc dot gnu.org
                   ` (12 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-07-23  1:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Paul Eggert <eggert at cs dot ucla.edu> ---
(In reply to Richard Biener from comment #13)
> Paul - can you test if this patch resolves the emacs issue?

Unfortunately not. Although the generated code differs, it's still the same bad
pattern. GDB's command 'disas decode_lisp_time' reports this:

     ...
     0x082a924c <+876>:   call   0x82a8140 <decode_ticks_hz>                    
  => 0x082a9251 <+881>:   fldl   0x8c(%esp)                                     
     0x082a9258 <+888>:   add    $0x10,%esp                                     
     0x082a925b <+891>:   fstpl  0x7c(%esp)
     ...                                       

where the 8-byte quantity being copied comes from the leading bytes of this
union:

  union c_time
  {
    struct ticks_hz { long long ticks; long long hz; } th;
    struct timespec ts;
    double d;
  };

and this union happens to contain th (of type struct ticks_hz) not d (of type
double).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (14 preceding siblings ...)
  2024-07-23  1:47 ` eggert at cs dot ucla.edu
@ 2024-07-23  6:23 ` rguenth at gcc dot gnu.org
  2024-07-23  6:28 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-23  6:23 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Paul Eggert from comment #16)
> (In reply to Richard Biener from comment #13)
> > Paul - can you test if this patch resolves the emacs issue?
> 
> Unfortunately not. Although the generated code differs, it's still the same
> bad pattern. GDB's command 'disas decode_lisp_time' reports this:
> 
>      ...
>      0x082a924c <+876>:   call   0x82a8140 <decode_ticks_hz>                
> 
>   => 0x082a9251 <+881>:   fldl   0x8c(%esp)                                 
> 
>      0x082a9258 <+888>:   add    $0x10,%esp                                 
> 
>      0x082a925b <+891>:   fstpl  0x7c(%esp)
>      ...                                       
> 
> where the 8-byte quantity being copied comes from the leading bytes of this
> union:
> 
>   union c_time
>   {
>     struct ticks_hz { long long ticks; long long hz; } th;
>     struct timespec ts;
>     double d;
>   };
> 
> and this union happens to contain th (of type struct ticks_hz) not d (of
> type double).

I suppose this happens even when building without LTO, so can you please
attach preprocessed source of the TU containing decode_lisp_time
(preprocessed with all the flags used when the issue appears)?  Maybe
it's really a duplicate of one of the related bugs (my patch didn't fix
all of them).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (15 preceding siblings ...)
  2024-07-23  6:23 ` rguenth at gcc dot gnu.org
@ 2024-07-23  6:28 ` rguenth at gcc dot gnu.org
  2024-07-23 12:55 ` jamborm at gcc dot gnu.org
                   ` (10 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-23  6:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Jambor from comment #15)
> Created attachment 58724 [details]
> simple (wip) fix
> 
> I'm wondering whether just simply something like this would not be enough. 
> I have looked at total scalarization and we will not replace a type found in
> the IL with another one there. Similarly, only propagation through
> assignments fiddles with existing types (when they are not aggregate) only
> when propagating from RHS to LHS and not the other way round.
> 
> If we want to be more aggressive, we can add a flag when the new predicate
> fails but there is a good bitwise_type_for_mode and then when the flag is
> set, use that type instead in analyze_access_subtree.
> 
> Note that so far I have only tested the attached patch with
>   make -k check-gcc RUNTESTFLAGS="tree-ssa.exp=*sra*.c"
>   make -k check-g++ RUNTESTFLAGS="dg.exp=*sra*.c"
>   make -k check-gcc RUNTESTFLAGS="dg.exp=*sra*.c"
> 
> I'll have a look at full test results tomorrow morning.

I think it should be OK to fix the wrong-code issue in this bug but it
prevents scalarization completely, likely at least failing
gcc.dg/tree-ssa/sra-6.c

Note the name of the predicate should probably reflect the problem,
like can_use_type_as_storage_for (tree storage_type, tree value_type)
or so.

With my patch I trieds to instead use a bit-pattern preserving load
similar as what we do with non-mode precision integer prevailing types.

Note I plan to add a new target hook to identify problematic FP modes.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (16 preceding siblings ...)
  2024-07-23  6:28 ` rguenth at gcc dot gnu.org
@ 2024-07-23 12:55 ` jamborm at gcc dot gnu.org
  2024-07-23 17:42 ` eggert at cs dot ucla.edu
                   ` (9 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: jamborm at gcc dot gnu.org @ 2024-07-23 12:55 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #19 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #18)
> (In reply to Martin Jambor from comment #15)
> > Created attachment 58724 [details]
> > simple (wip) fix
> > 
> > I'm wondering whether just simply something like this would not be enough. 
> > I have looked at total scalarization and we will not replace a type found in
> > the IL with another one there. Similarly, only propagation through
> > assignments fiddles with existing types (when they are not aggregate) only
> > when propagating from RHS to LHS and not the other way round.
> > 
> > If we want to be more aggressive, we can add a flag when the new predicate
> > fails but there is a good bitwise_type_for_mode and then when the flag is
> > set, use that type instead in analyze_access_subtree.
> > 
> > Note that so far I have only tested the attached patch with
> >   make -k check-gcc RUNTESTFLAGS="tree-ssa.exp=*sra*.c"
> >   make -k check-g++ RUNTESTFLAGS="dg.exp=*sra*.c"
> >   make -k check-gcc RUNTESTFLAGS="dg.exp=*sra*.c"
> > 
> > I'll have a look at full test results tomorrow morning.
> 
> I think it should be OK to fix the wrong-code issue in this bug but it
> prevents scalarization completely, likely at least failing
> gcc.dg/tree-ssa/sra-6.c

I did check that even yesterday night and it passes but g++.dg/vect/pr64410.cc
and gcc.dg/tree-ssa/pr32964.c fail.  I'm investigating.

> 
> Note the name of the predicate should probably reflect the problem,
> like can_use_type_as_storage_for (tree storage_type, tree value_type)
> or so.

Yes, I'm aware it is less than ideal so far.  I was not sure what the final
version would be like.  THe version from yesterday unnecessarily pessimizes
cases where the "other" type is a structure containing just the float or one
element array, or a combination, that at least should be addresses.

> 
> With my patch I trieds to instead use a bit-pattern preserving load
> similar as what we do with non-mode precision integer prevailing types.
> 
> Note I plan to add a new target hook to identify problematic FP modes.

That would be super-helpful.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (17 preceding siblings ...)
  2024-07-23 12:55 ` jamborm at gcc dot gnu.org
@ 2024-07-23 17:42 ` eggert at cs dot ucla.edu
  2024-07-23 17:46 ` eggert at cs dot ucla.edu
                   ` (8 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-07-23 17:42 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #20 from Paul Eggert <eggert at cs dot ucla.edu> ---
Created attachment 58739
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58739&action=edit
preprocessed Emacs source code illustrating the bug

Here is the gzip-compressed text of the Emacs source code illustrating the bug.
I generated it via the following commands on an AMD Phenom II X4 910e.

gcc -m32 -march=native -E -Demacs -I. -I. -I../lib -I../lib -isystem
/usr/include/libpng16 -DWITH_GZFILEOP -isystem /usr/include/libxml2
-DWITH_GZFILEOP -isystem /usr/include/webp -MMD -MF deps/timefns.d -MP -Werror
-fanalyzer -fstrict-flex-arrays -Wall -Warith-conversion -Wdate-time
-Wdisabled-optimization -Wdouble-promotion -Wduplicated-cond -Wextra
-Wformat-signedness -Wflex-array-member-not-at-end -Winit-self -Winvalid-pch
-Wlogical-op -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes
-Wmissing-variable-declarations -Wnested-externs -Wnull-dereference
-Wold-style-definition -Wopenmp-simd -Wpacked -Wpointer-arith
-Wstrict-flex-arrays -Wstrict-prototypes -Wsuggest-attribute=format
-Wsuggest-attribute=malloc -Wsuggest-attribute=noreturn -Wsuggest-final-methods
-Wsuggest-final-types -Wtrampolines -Wuninitialized -Wunknown-pragmas
-Wunused-macros -Wvariadic-macros -Wvector-operation-performance
-Wwrite-strings -Warray-bounds=2 -Wattribute-alias=2 -Wformat=2
-Wformat-truncation=2 -Wimplicit-fallthrough=5 -Wshift-overflow=2
-Wuse-after-free=3 -Wvla-larger-than=4031 -Wno-analyzer-malloc-leak
-Wredundant-decls -Wno-missing-field-initializers -Wno-override-init
-Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral
-Wno-bidi-chars -Wno-analyzer-fd-leak -g3 -O2 timefns.c >timefns.i

gzip -9n timefns.i

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (18 preceding siblings ...)
  2024-07-23 17:42 ` eggert at cs dot ucla.edu
@ 2024-07-23 17:46 ` eggert at cs dot ucla.edu
  2024-07-23 17:48 ` eggert at cs dot ucla.edu
                   ` (7 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-07-23 17:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #21 from Paul Eggert <eggert at cs dot ucla.edu> ---
Created attachment 58740
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58740&action=edit
Assembly-language illustrating wrong code

Here is the assembly language output (gzip compressed) of the wrong code. The
wrong code starts at line 3025, where fldl/fstpl is incorrectly used to copy a
64-bit int.

I used the same gcc command as before, except with -S instead of -E, and I
omitted  to save space in the .s file.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (19 preceding siblings ...)
  2024-07-23 17:46 ` eggert at cs dot ucla.edu
@ 2024-07-23 17:48 ` eggert at cs dot ucla.edu
  2024-07-24 12:29 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-07-23 17:48 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #22 from Paul Eggert <eggert at cs dot ucla.edu> ---
(In reply to Richard Biener from comment #17)

> I suppose this happens even when building without LTO, so can you please
> attach preprocessed source of the TU containing decode_lisp_time
> (preprocessed with all the flags used when the issue appears)?  Maybe
> it's really a duplicate of one of the related bugs (my patch didn't fix
> all of them).

Thanks, I've attached the preprocessed source and wrong code in my previous two
comments. You're correct that it happens without LTO.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (20 preceding siblings ...)
  2024-07-23 17:48 ` eggert at cs dot ucla.edu
@ 2024-07-24 12:29 ` rguenth at gcc dot gnu.org
  2024-07-24 19:33 ` jamborm at gcc dot gnu.org
                   ` (5 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-07-24 12:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #23 from Richard Biener <rguenth at gcc dot gnu.org> ---
So it's from

    return (struct form_time)
      {
 .form = TIMEFORM_HI_LO,
 .time = decode_ticks_hz (specified_time, make_fixnum (1), cform)
      };

with

union c_time
{
  struct ticks_hz th;
  struct timespec ts;
  double d;
};

struct form_time
{
  enum timeform form;
  union c_time time;
};

since there's also

   .form = TIMEFORM_FLOAT,
   .time = decode_float_time (d, cform)

and that one is inlined we're likely only seeing an access to the double
union component and otherwise aggregate union accesses.

Still needs a reduction.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (21 preceding siblings ...)
  2024-07-24 12:29 ` rguenth at gcc dot gnu.org
@ 2024-07-24 19:33 ` jamborm at gcc dot gnu.org
  2024-08-20 12:11 ` jamborm at gcc dot gnu.org
                   ` (4 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: jamborm at gcc dot gnu.org @ 2024-07-24 19:33 UTC (permalink / raw)
  To: gcc-bugs

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #58724|0                           |1
        is obsolete|                            |

--- Comment #24 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 58752
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58752&action=edit
Another wip patch

To give sort-of a status update, this is the current state of my WIP fix.  It
clearly needs more thinking about the reverse storage cases and still fails
g++.dg/vect/pr64410.cc - but I'm afraid that would require a target hook to
identify problematic FP modes.  Otherwise it passes bootstrap and testsuite on
x86_64-linux.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (22 preceding siblings ...)
  2024-07-24 19:33 ` jamborm at gcc dot gnu.org
@ 2024-08-20 12:11 ` jamborm at gcc dot gnu.org
  2024-08-21 12:49 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: jamborm at gcc dot gnu.org @ 2024-08-20 12:11 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #25 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I have proposed a patch on the mailing list:
https://inbox.sourceware.org/gcc-patches/ri6ed6kntue.fsf@virgil.suse.cz/T/#u

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (23 preceding siblings ...)
  2024-08-20 12:11 ` jamborm at gcc dot gnu.org
@ 2024-08-21 12:49 ` cvs-commit at gcc dot gnu.org
  2024-08-21 13:26 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  27 siblings, 0 replies; 29+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-08-21 12:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #26 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:f577959f420ae404f99f630dadc1c0370734d0da

commit r15-3070-gf577959f420ae404f99f630dadc1c0370734d0da
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Aug 21 14:49:11 2024 +0200

    sra: Avoid risking x87 magling binary representation of a replacement (PR
58416)

    PR 58416 shows that storing non-floating point data to floating point
    scalar registers can lead to miscompilations when the data is
    normalized or otherwise processed upon loading to a register.  To
    avoid that risk, this patch detects situations where we have multiple
    types and a we decide to represent the data in a type with a mode that
    is known to not be able to transfer actual bits reliably using the new
    TARGET_MODE_CAN_TRANSFER_BITS hook.

    gcc/ChangeLog:

    2024-08-19  Martin Jambor  <mjambor@suse.cz>

            PR target/58416
            * tree-sra.cc (types_risk_mangled_binary_repr_p): New function.
            (sort_and_splice_var_accesses): Use it.
            (propagate_subaccesses_from_rhs): Likewise.

    gcc/testsuite/ChangeLog:

    2024-08-19  Martin Jambor  <mjambor@suse.cz>

            PR target/58416
            * gcc.dg/torture/pr58416.c: New test.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (24 preceding siblings ...)
  2024-08-21 12:49 ` cvs-commit at gcc dot gnu.org
@ 2024-08-21 13:26 ` rguenth at gcc dot gnu.org
  2024-08-22  6:39 ` eggert at cs dot ucla.edu
  2024-08-22  6:41 ` sjames at gcc dot gnu.org
  27 siblings, 0 replies; 29+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-08-21 13:26 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |15.0

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (25 preceding siblings ...)
  2024-08-21 13:26 ` rguenth at gcc dot gnu.org
@ 2024-08-22  6:39 ` eggert at cs dot ucla.edu
  2024-08-22  6:41 ` sjames at gcc dot gnu.org
  27 siblings, 0 replies; 29+ messages in thread
From: eggert at cs dot ucla.edu @ 2024-08-22  6:39 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #28 from Paul Eggert <eggert at cs dot ucla.edu> ---
Thanks for the fix. I updated Emacs to no longer work around the bug when GCC
15+ is being used, here:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3d1d4f109ed4115256a08c74eeb704259d91c9f4

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Bug target/58416] Incorrect x87-based union copying code
  2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
                   ` (26 preceding siblings ...)
  2024-08-22  6:39 ` eggert at cs dot ucla.edu
@ 2024-08-22  6:41 ` sjames at gcc dot gnu.org
  27 siblings, 0 replies; 29+ messages in thread
From: sjames at gcc dot gnu.org @ 2024-08-22  6:41 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #29 from Sam James <sjames at gcc dot gnu.org> ---
It wasn't clear to me if your case was fixed (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58416#c23) or if it needed
reduction.

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2024-08-22  6:41 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-13 19:56 [Bug target/58416] New: Incorrect x87-based union copying code stichnot at google dot com
2013-09-13 20:23 ` [Bug target/58416] " hjl.tools at gmail dot com
2013-09-16 10:28 ` rguenth at gcc dot gnu.org
2013-09-16 13:21 ` ubizjak at gmail dot com
2013-09-16 15:49 ` joseph at codesourcery dot com
2024-02-20 10:05 ` rguenth at gcc dot gnu.org
2024-02-20 10:07 ` rguenth at gcc dot gnu.org
2024-02-20 10:12 ` pinskia at gcc dot gnu.org
2024-07-18 14:16 ` akrl at gcc dot gnu.org
2024-07-19 12:34 ` rguenth at gcc dot gnu.org
2024-07-20 14:30 ` eggert at cs dot ucla.edu
2024-07-21  8:58 ` rguenth at gcc dot gnu.org
2024-07-21  8:58 ` rguenth at gcc dot gnu.org
2024-07-21 10:21 ` rguenth at gcc dot gnu.org
2024-07-22 21:09 ` jamborm at gcc dot gnu.org
2024-07-23  1:47 ` eggert at cs dot ucla.edu
2024-07-23  6:23 ` rguenth at gcc dot gnu.org
2024-07-23  6:28 ` rguenth at gcc dot gnu.org
2024-07-23 12:55 ` jamborm at gcc dot gnu.org
2024-07-23 17:42 ` eggert at cs dot ucla.edu
2024-07-23 17:46 ` eggert at cs dot ucla.edu
2024-07-23 17:48 ` eggert at cs dot ucla.edu
2024-07-24 12:29 ` rguenth at gcc dot gnu.org
2024-07-24 19:33 ` jamborm at gcc dot gnu.org
2024-08-20 12:11 ` jamborm at gcc dot gnu.org
2024-08-21 12:49 ` cvs-commit at gcc dot gnu.org
2024-08-21 13:26 ` rguenth at gcc dot gnu.org
2024-08-22  6:39 ` eggert at cs dot ucla.edu
2024-08-22  6:41 ` sjames at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).