Patchwork GCC 4.6.x miscompiling arm-linux?

login
register
mail settings
Submitter Mikael Pettersson
Date Sept. 11, 2012, 2:10 p.m.
Message ID <20559.17990.950313.939717@pilspetsen.it.uu.se>
Download mbox | patch
Permalink /patch/1438281/
State New, archived
Headers show

Comments

Mikael Pettersson - Sept. 11, 2012, 2:10 p.m.
David Jander writes:
 > On Tue, 11 Sep 2012 14:53:35 +0200
 > Mikael Pettersson <mikpe@it.uu.se> wrote:
 > 
 > > David Jander writes:
 > >  > On Tue, 11 Sep 2012 13:35:40 +0200
 > >  > Mikael Pettersson <mikpe@it.uu.se> wrote:
 > >  > 
 > >  > > David Jander writes:
 > >  > >  > > I can make the patches available if there's confirmation that a vanilla
 > >  > >  > > upstream gcc-4.6.3 doesn't work.
 > >  > >  > 
 > >  > >  > I am pretty sure this is the case... do you have a patch series that you can
 > >  > >  > easily tar and mail to me? I'd like to try those patches with OSELAS, to see
 > >  > >  > if I can indeed build a gcc-4.6.3 toolchain that generates correct code.... I
 > >  > >  > already know that I can generate one that doesn't ;-)
 > >  > >  > 
 > >  > > ...
 > >  > >  > > If you're sure no add-on patches were applied, then yes please do.
 > >  > >  > 
 > >  > >  > Pretty sure, but not 100%, so I'd like to try your patches first if you don't
 > >  > >  > mind....
 > >  > > 
 > >  > > And I'd like to confirm first. Please tell us the following:
 > >  > > 
 > >  > >  > >  > > 2: include the output of gcc -v which tells how that gcc was configured,
 > >  > 
 > >  > Using built-in specs.
 > >  > COLLECT_GCC=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/bin/arm-v5te-linux-gnueabi-gcc-4.6.3
 > >  > COLLECT_LTO_WRAPPER=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/libexec/gcc/arm-v5te-linux-gnueabi/4.6.3/lto-wrapper
 > >  > Target: arm-v5te-linux-gnueabi
 > >  > Configured with: /home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te-linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/build-cross/gcc-4.6.3/configure --target=arm-v5te-linux-gnueabi --with-sysroot=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3
 > >  > -glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-arm-v5te-linux-gnueabi --disable-multilib --with-float=soft --with-fpu=vfp --with-cpu=arm926ej-s --enable-__cxa_atexit --disable-sjlj-exceptions --disable-nls --disable-decimal-float --disable-fixed-point --disable-win32-registry --enable-symvers=gnu --with-pkgversion=OSELAS.Toolchain-2011.11.1 --with-system-zlib --with-gmp=/home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te
 > >  > -linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-host --with-mpfr=/home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te-linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-host --prefix=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized --enable-languages=c,c++ --enable-threads=posix --enable-c99 --enable-long-long --enable-libstdcxx-debug --enable-profile --enable-shared --disable-libssp --enable-checking=release
 > >  > Thread model: posix
 > >  > gcc version 4.6.3 (OSELAS.Toolchain-2011.11.1)
 > >  > 
 > >  > >  > >  > > 3: give the exact set of gcc options used then compiling the test case.
 > >  > 
 > >  > If I type this in a terminal:
 > >  > 
 > >  > 
 > >  > $ /opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/bin/arm-v5te-linux-gnueabi-gcc-4.6.3 -Os -S -o - -x c -
 > >  > 
 > >  > struct flexcan_regs {
 > >  >         unsigned int mcr;
 > >  >         unsigned int rxfgmask;
 > >  > };
 > >  > 
 > >  > #define flexcan_read(a) (*(volatile unsigned int *)(a))
 > >  > #define flexcan_write(v,a)      (*(volatile unsigned int *)(a) = (v))
 > >  > 
 > >  > int flexcan_chip_start(int ver, struct flexcan_regs *regs)
 > >  > {
 > >  >         flexcan_write(0, &regs->mcr);
 > >  > 
 > >  >         if (ver >= 10)
 > >  >                 flexcan_write(0, &regs->rxfgmask);
 > >  > 
 > >  >         return 0;
 > >  > }
 > >  > 
 > >  > 
 > >  > I get this output after hitting <CTRL-D>:
 > >  > 
 > >  >         .cpu arm926ej-s
 > >  >         .fpu softvfp
 > >  >         .eabi_attribute 20, 1
 > >  >         .eabi_attribute 21, 1
 > >  >         .eabi_attribute 23, 3
 > >  >         .eabi_attribute 24, 1
 > >  >         .eabi_attribute 25, 1
 > >  >         .eabi_attribute 26, 2
 > >  >         .eabi_attribute 30, 4
 > >  >         .eabi_attribute 18, 4
 > >  >         .file   ""
 > >  >         .text
 > >  >         .align  2
 > >  >         .global flexcan_chip_start
 > >  >         .type   flexcan_chip_start, %function
 > >  > flexcan_chip_start:
 > >  >         @ args = 0, pretend = 0, frame = 0
 > >  >         @ frame_needed = 0, uses_anonymous_args = 0
 > >  >         @ link register save eliminated.
 > >  >         mov     r3, #0
 > >  >         cmp     r0, #9
 > >  >         str     r3, [r1, #0]
 > >  >         ldrle   r3, [r1, #4]
 > >  >         mov     r0, #0
 > >  >         str     r3, [r1, #4]
 > >  >         bx      lr
 > >  >         .size   flexcan_chip_start, .-flexcan_chip_start
 > >  >         .ident  "GCC: (OSELAS.Toolchain-2011.11.1) 4.6.3"
 > >  >         .section        .note.GNU-stack,"",%progbits
 > >  > 
 > >  > 
 > >  > Do you need more information?
 > > 
 > > No, I can reproduce the bug with vanilla gcc-4.6.3; vanilla 4.7.1 and 4.5.4 are Ok.
 > > 
 > > I'll bisect my 4.6.3 patch series to see which patch fixes it.
 > 
 > Great. Thanks a lot for your help so far. Looking forward to see what fixes
 > this issue. Are you implying that you will also file the bug (and possible
 > patch) with gcc.gnu.org, or do you prefer me to do that?

This is a known bug: <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52445>.
It was reported and fixed in gcc trunk on March 1 this year, but missed
the gcc-4.6.3 release made the same day (and frozen a week or so before),
and it hasn't been applied to gcc-4.6.4 branch yet either.

I've been using and testing the fix in my own gcc-4.6 branch since March 4
without regressions. I'm attaching my backport of the fix below.

I'll ping gcc upstream about getting this into gcc-4.6.4.

/Mikael
[backport from gcc-4.8/trunk r184743 <mikpe@it.uu.se>]

gcc/

2012-03-01  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/52445
	* tree-ssa-phiopt.c (struct name_to_bb): Remove ssa_name field,
	add ssa_name_ver, offset and size fields and change store field
	to bool.
	(name_to_bb_hash, name_to_bb_eq): Adjust for the above changes.
	(add_or_mark_expr): Likewise.  Only consider previous stores
	with the same size and offset.
	(nt_init_block): Only look at gimple_assign_single_p stmts,
	doesn't look at rhs2.

gcc/testsuite/

2012-03-01  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/52445
	* gcc.dg/pr52445.c: New test.
David Jander - Sept. 13, 2012, 8:38 a.m.
On Tue, 11 Sep 2012 16:10:14 +0200
Mikael Pettersson <mikpe@it.uu.se> wrote:

> David Jander writes:
>  > On Tue, 11 Sep 2012 14:53:35 +0200
>  > Mikael Pettersson <mikpe@it.uu.se> wrote:
>  > 
>  > > David Jander writes:
>  > >  > On Tue, 11 Sep 2012 13:35:40 +0200
>  > >  > Mikael Pettersson <mikpe@it.uu.se> wrote:
>  > >  > 
>  > >  > > David Jander writes:
>  > >  > >  > > I can make the patches available if there's confirmation that a vanilla
>  > >  > >  > > upstream gcc-4.6.3 doesn't work.
>  > >  > >  > 
>  > >  > >  > I am pretty sure this is the case... do you have a patch series that you can
>  > >  > >  > easily tar and mail to me? I'd like to try those patches with OSELAS, to see
>  > >  > >  > if I can indeed build a gcc-4.6.3 toolchain that generates correct code.... I
>  > >  > >  > already know that I can generate one that doesn't ;-)
>  > >  > >  > 
>  > >  > > ...
>  > >  > >  > > If you're sure no add-on patches were applied, then yes please do.
>  > >  > >  > 
>  > >  > >  > Pretty sure, but not 100%, so I'd like to try your patches first if you don't
>  > >  > >  > mind....
>  > >  > > 
>  > >  > > And I'd like to confirm first. Please tell us the following:
>  > >  > > 
>  > >  > >  > >  > > 2: include the output of gcc -v which tells how that gcc was configured,
>  > >  > 
>  > >  > Using built-in specs.
>  > >  > COLLECT_GCC=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/bin/arm-v5te-linux-gnueabi-gcc-4.6.3
>  > >  > COLLECT_LTO_WRAPPER=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/libexec/gcc/arm-v5te-linux-gnueabi/4.6.3/lto-wrapper
>  > >  > Target: arm-v5te-linux-gnueabi
>  > >  > Configured with: /home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te-linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/build-cross/gcc-4.6.3/configure --target=arm-v5te-linux-gnueabi --with-sysroot=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3
>  > >  > -glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-arm-v5te-linux-gnueabi --disable-multilib --with-float=soft --with-fpu=vfp --with-cpu=arm926ej-s --enable-__cxa_atexit --disable-sjlj-exceptions --disable-nls --disable-decimal-float --disable-fixed-point --disable-win32-registry --enable-symvers=gnu --with-pkgversion=OSELAS.Toolchain-2011.11.1 --with-system-zlib --with-gmp=/home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te
>  > >  > -linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-host --with-mpfr=/home/djander/ptxdist/OSELAS.Toolchain-2011.11.1/platform-arm-v5te-linux-gnueabi-gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/sysroot-host --prefix=/opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized --enable-languages=c,c++ --enable-threads=posix --enable-c99 --enable-long-long --enable-libstdcxx-debug --enable-profile --enable-shared --disable-libssp --enable-checking=release
>  > >  > Thread model: posix
>  > >  > gcc version 4.6.3 (OSELAS.Toolchain-2011.11.1)
>  > >  > 
>  > >  > >  > >  > > 3: give the exact set of gcc options used then compiling the test case.
>  > >  > 
>  > >  > If I type this in a terminal:
>  > >  > 
>  > >  > 
>  > >  > $ /opt/OSELAS.Toolchain-2011.11.1/arm-v5te-linux-gnueabi/gcc-4.6.3-glibc-2.14.1-binutils-2.21.1a-kernel-2.6.39-sanitized/bin/arm-v5te-linux-gnueabi-gcc-4.6.3 -Os -S -o - -x c -
>  > >  > 
>  > >  > struct flexcan_regs {
>  > >  >         unsigned int mcr;
>  > >  >         unsigned int rxfgmask;
>  > >  > };
>  > >  > 
>  > >  > #define flexcan_read(a) (*(volatile unsigned int *)(a))
>  > >  > #define flexcan_write(v,a)      (*(volatile unsigned int *)(a) = (v))
>  > >  > 
>  > >  > int flexcan_chip_start(int ver, struct flexcan_regs *regs)
>  > >  > {
>  > >  >         flexcan_write(0, &regs->mcr);
>  > >  > 
>  > >  >         if (ver >= 10)
>  > >  >                 flexcan_write(0, &regs->rxfgmask);
>  > >  > 
>  > >  >         return 0;
>  > >  > }
>  > >  > 
>  > >  > 
>  > >  > I get this output after hitting <CTRL-D>:
>  > >  > 
>  > >  >         .cpu arm926ej-s
>  > >  >         .fpu softvfp
>  > >  >         .eabi_attribute 20, 1
>  > >  >         .eabi_attribute 21, 1
>  > >  >         .eabi_attribute 23, 3
>  > >  >         .eabi_attribute 24, 1
>  > >  >         .eabi_attribute 25, 1
>  > >  >         .eabi_attribute 26, 2
>  > >  >         .eabi_attribute 30, 4
>  > >  >         .eabi_attribute 18, 4
>  > >  >         .file   ""
>  > >  >         .text
>  > >  >         .align  2
>  > >  >         .global flexcan_chip_start
>  > >  >         .type   flexcan_chip_start, %function
>  > >  > flexcan_chip_start:
>  > >  >         @ args = 0, pretend = 0, frame = 0
>  > >  >         @ frame_needed = 0, uses_anonymous_args = 0
>  > >  >         @ link register save eliminated.
>  > >  >         mov     r3, #0
>  > >  >         cmp     r0, #9
>  > >  >         str     r3, [r1, #0]
>  > >  >         ldrle   r3, [r1, #4]
>  > >  >         mov     r0, #0
>  > >  >         str     r3, [r1, #4]
>  > >  >         bx      lr
>  > >  >         .size   flexcan_chip_start, .-flexcan_chip_start
>  > >  >         .ident  "GCC: (OSELAS.Toolchain-2011.11.1) 4.6.3"
>  > >  >         .section        .note.GNU-stack,"",%progbits
>  > >  > 
>  > >  > 
>  > >  > Do you need more information?
>  > > 
>  > > No, I can reproduce the bug with vanilla gcc-4.6.3; vanilla 4.7.1 and 4.5.4 are Ok.
>  > > 
>  > > I'll bisect my 4.6.3 patch series to see which patch fixes it.
>  > 
>  > Great. Thanks a lot for your help so far. Looking forward to see what fixes
>  > this issue. Are you implying that you will also file the bug (and possible
>  > patch) with gcc.gnu.org, or do you prefer me to do that?
> 
> This is a known bug: <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52445>.
> It was reported and fixed in gcc trunk on March 1 this year, but missed
> the gcc-4.6.3 release made the same day (and frozen a week or so before),
> and it hasn't been applied to gcc-4.6.4 branch yet either.
> 
> I've been using and testing the fix in my own gcc-4.6 branch since March 4
> without regressions. I'm attaching my backport of the fix below.
> 
> I'll ping gcc upstream about getting this into gcc-4.6.4.

Thanks a lot for this patch.
I can confirm that the bug is gone, and no other problems have appeared so
far. Successfully recompiled kernel and the entire userspace (ptxdist) with
the patched toolchain, and all seems well.

Best regards,

Patch

--- gcc-4.6.3/gcc/testsuite/gcc.dg/pr52445.c.~1~	1970-01-01 01:00:00.000000000 +0100
+++ gcc-4.6.3/gcc/testsuite/gcc.dg/pr52445.c	2012-03-04 16:48:50.000000000 +0100
@@ -0,0 +1,15 @@ 
+/* PR tree-optimization/52445 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-cselim -fdump-tree-cselim" } */
+
+void
+foo (char *buf, unsigned long len)
+{
+  buf[0] = '\n';
+  if (len > 1)
+    buf[1] = '\0';	/* We can't cselim "optimize" this, while
+			   buf[0] doesn't trap, buf[1] could.  */
+}
+
+/* { dg-final { scan-tree-dump-not "cstore\." "cselim" } } */
+/* { dg-final { cleanup-tree-dump "cselim" } } */
--- gcc-4.6.3/gcc/tree-ssa-phiopt.c.~1~	2010-11-03 16:18:50.000000000 +0100
+++ gcc-4.6.3/gcc/tree-ssa-phiopt.c	2012-03-04 16:48:50.000000000 +0100
@@ -1050,9 +1050,10 @@  abs_replacement (basic_block cond_bb, ba
    same accesses.  */
 struct name_to_bb
 {
-  tree ssa_name;
+  unsigned int ssa_name_ver;
+  bool store;
+  HOST_WIDE_INT offset, size;
   basic_block bb;
-  unsigned store : 1;
 };
 
 /* The hash table for remembering what we've seen.  */
@@ -1061,23 +1062,26 @@  static htab_t seen_ssa_names;
 /* The set of MEM_REFs which can't trap.  */
 static struct pointer_set_t *nontrap_set;
 
-/* The hash function, based on the pointer to the pointer SSA_NAME.  */
+/* The hash function.  */
 static hashval_t
 name_to_bb_hash (const void *p)
 {
-  const_tree n = ((const struct name_to_bb *)p)->ssa_name;
-  return htab_hash_pointer (n) ^ ((const struct name_to_bb *)p)->store;
+  const struct name_to_bb *n = (const struct name_to_bb *) p;
+  return n->ssa_name_ver ^ (((hashval_t) n->store) << 31)
+         ^ (n->offset << 6) ^ (n->size << 3);
 }
 
-/* The equality function of *P1 and *P2.  SSA_NAMEs are shared, so
-   it's enough to simply compare them for equality.  */
+/* The equality function of *P1 and *P2.  */
 static int
 name_to_bb_eq (const void *p1, const void *p2)
 {
   const struct name_to_bb *n1 = (const struct name_to_bb *)p1;
   const struct name_to_bb *n2 = (const struct name_to_bb *)p2;
 
-  return n1->ssa_name == n2->ssa_name && n1->store == n2->store;
+  return n1->ssa_name_ver == n2->ssa_name_ver
+         && n1->store == n2->store
+         && n1->offset == n2->offset
+         && n1->size == n2->size;
 }
 
 /* We see the expression EXP in basic block BB.  If it's an interesting
@@ -1089,8 +1093,12 @@  static void
 add_or_mark_expr (basic_block bb, tree exp,
 		  struct pointer_set_t *nontrap, bool store)
 {
+  HOST_WIDE_INT size;
+
   if (TREE_CODE (exp) == MEM_REF
-      && TREE_CODE (TREE_OPERAND (exp, 0)) == SSA_NAME)
+      && TREE_CODE (TREE_OPERAND (exp, 0)) == SSA_NAME
+      && host_integerp (TREE_OPERAND (exp, 1), 0)
+      && (size = int_size_in_bytes (TREE_TYPE (exp))) > 0)
     {
       tree name = TREE_OPERAND (exp, 0);
       struct name_to_bb map;
@@ -1100,9 +1108,12 @@  add_or_mark_expr (basic_block bb, tree e
 
       /* Try to find the last seen MEM_REF through the same
          SSA_NAME, which can trap.  */
-      map.ssa_name = name;
+      map.ssa_name_ver = SSA_NAME_VERSION (name);
       map.bb = 0;
       map.store = store;
+      map.offset = tree_low_cst (TREE_OPERAND (exp, 1), 0);
+      map.size = size;
+
       slot = htab_find_slot (seen_ssa_names, &map, INSERT);
       n2bb = (struct name_to_bb *) *slot;
       if (n2bb)
@@ -1125,9 +1136,11 @@  add_or_mark_expr (basic_block bb, tree e
 	  else
 	    {
 	      n2bb = XNEW (struct name_to_bb);
-	      n2bb->ssa_name = name;
+	      n2bb->ssa_name_ver = SSA_NAME_VERSION (name);
 	      n2bb->bb = bb;
 	      n2bb->store = store;
+	      n2bb->offset = map.offset;
+	      n2bb->size = size;
 	      *slot = n2bb;
 	    }
 	}
@@ -1147,13 +1160,10 @@  nt_init_block (struct dom_walk_data *dat
     {
       gimple stmt = gsi_stmt (gsi);
 
-      if (is_gimple_assign (stmt))
+      if (gimple_assign_single_p (stmt))
 	{
 	  add_or_mark_expr (bb, gimple_assign_lhs (stmt), nontrap_set, true);
 	  add_or_mark_expr (bb, gimple_assign_rhs1 (stmt), nontrap_set, false);
-	  if (get_gimple_rhs_num_ops (gimple_assign_rhs_code (stmt)) > 1)
-	    add_or_mark_expr (bb, gimple_assign_rhs2 (stmt), nontrap_set,
-			      false);
 	}
     }
 }