From patchwork Tue Sep 11 14:10:14 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mikael Pettersson X-Patchwork-Id: 1438281 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id EA742DFAF3 for ; Tue, 11 Sep 2012 14:16:03 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TBRCN-0003fH-6k; Tue, 11 Sep 2012 14:12:19 +0000 Received: from smtp1.uu.se ([2001:6b0:b:242:130:238:7:54]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TBRCG-0003d6-4w for linux-arm-kernel@lists.infradead.org; Tue, 11 Sep 2012 14:12:15 +0000 X-Virus-Scanned: amavisd-new at uu.se X-Spam-Flag: NO X-Spam-Score: -3.835 X-Spam-Level: X-Spam-Status: No, score=-3.835 tagged_above=-9999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-1.935] autolearn=unavailable Received: from pilspetsen.it.uu.se (pilspetsen.it.uu.se [130.238.18.39]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by bornea.its.uu.se (Postfix) with ESMTPS id 0657F3808D; Tue, 11 Sep 2012 16:11:06 +0200 (CEST) Received: (from mikpe@localhost) by pilspetsen.it.uu.se (8.14.5+Sun/8.14.5) id q8BEAFIJ005219; Tue, 11 Sep 2012 16:10:15 +0200 (MEST) X-Authentication-Warning: pilspetsen.it.uu.se: mikpe set sender to mikpe@it.uu.se using -f MIME-Version: 1.0 Message-ID: <20559.17990.950313.939717@pilspetsen.it.uu.se> Date: Tue, 11 Sep 2012 16:10:14 +0200 From: Mikael Pettersson To: David Jander Subject: Re: GCC 4.6.x miscompiling arm-linux? In-Reply-To: <20120911154308.3355a480@archvile> References: <20120910171654.1d4972b2@archvile> <20120911092753.7b8315d4@archvile> <20558.61974.299424.536837@pilspetsen.it.uu.se> <20120911104925.3461603e@archvile> <20559.1844.786507.216580@pilspetsen.it.uu.se> <20120911123743.29e0ed40@archvile> <20559.8716.124212.451155@pilspetsen.it.uu.se> <20120911135212.187bd224@archvile> <20559.13391.264611.307105@pilspetsen.it.uu.se> <20120911154308.3355a480@archvile> X-Mailer: VM 7.17 under Emacs 20.7.1 X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.2 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.3 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Matthew Leach , Mikael Pettersson , Michael Olbrich , Sascha Hauer , Marc Kleine-Budde , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org David Jander writes: > On Tue, 11 Sep 2012 14:53:35 +0200 > Mikael Pettersson wrote: > > > David Jander writes: > > > On Tue, 11 Sep 2012 13:35:40 +0200 > > > Mikael Pettersson 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, ®s->mcr); > > > > > > if (ver >= 10) > > > flexcan_write(0, ®s->rxfgmask); > > > > > > return 0; > > > } > > > > > > > > > I get this output after hitting : > > > > > > .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: . 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 ] gcc/ 2012-03-01 Jakub Jelinek 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 PR tree-optimization/52445 * gcc.dg/pr52445.c: New test. --- 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); } } }