From patchwork Wed Aug 15 00:20:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 177522 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id E7AE12C0089 for ; Wed, 15 Aug 2012 10:21:06 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1345594867; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:Received:Date:Message-Id:From:To: Subject:MIME-Version:Content-Type:Content-Transfer-Encoding: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=hgTlwep8qMIWiOdqM+yb np51O9g=; b=Bu6FvO0/S+Y/4Lm6N7jQoaRFWSLerCLuAVuWqFhub8tZCuliNblb DZ29If91TnSVfVLoC9RraeZPUUhOqnklIFl1Iu0E5EXQFiagt6MZOebdT0vunNCl EJIIp3KXnisPMgB4gG6wrtTn3SKx46BO7LhO5Gwo+wZQKkgh8UYFbIs= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:Received:Date:Message-Id:From:To:Subject:MIME-Version:Content-Type:Content-Transfer-Encoding:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=fPtZHlKSWBliblk+LItqz0x4s6ZYDJmks4JXAA4POq5BTeZkymC9zMnQQhX8FD JrxOARbxYNrLzjSGmSnoE3zenW0YyZQ9pZ0X3iNmRRbF9OSPPItjwBfqMQtwLNwe K2Xm9DqlxZefkpysStlh2+nwMpz1MDVCZKCZ699gDUBjU=; Received: (qmail 24884 invoked by alias); 15 Aug 2012 00:20:59 -0000 Received: (qmail 24865 invoked by uid 22791); 15 Aug 2012 00:20:56 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_NO, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from anubis.se.axis.com (HELO anubis.se.axis.com) (195.60.68.12) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 15 Aug 2012 00:20:42 +0000 Received: from localhost (localhost [127.0.0.1]) by anubis.se.axis.com (Postfix) with ESMTP id 69A3319DD0 for ; Wed, 15 Aug 2012 02:20:40 +0200 (CEST) Received: from anubis.se.axis.com ([127.0.0.1]) by localhost (anubis.se.axis.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id MIoCiRqz1tom for ; Wed, 15 Aug 2012 02:20:38 +0200 (CEST) Received: from thoth.se.axis.com (thoth.se.axis.com [10.0.2.173]) by anubis.se.axis.com (Postfix) with ESMTP id 53D7519D8B for ; Wed, 15 Aug 2012 02:20:38 +0200 (CEST) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by thoth.se.axis.com (Postfix) with ESMTP id 51B40340CA; Wed, 15 Aug 2012 02:20:38 +0200 (CEST) Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id q7F0Kcd0006572; Wed, 15 Aug 2012 02:20:38 +0200 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id q7F0KbaV006568; Wed, 15 Aug 2012 02:20:37 +0200 Date: Wed, 15 Aug 2012 02:20:37 +0200 Message-Id: <201208150020.q7F0KbaV006568@ignucius.se.axis.com> From: Hans-Peter Nilsson To: gcc-patches@gcc.gnu.org Subject: [RFA:] fix PR54261, reverse operator emitted for compare_and_swap-libfunc targets MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org If a target implements (some) atomics by only setting sync_compare_and_swap_optab libfuncs (see cris.c:cris_init_libfuncs), a code-path less travelled is used. There's a bug there, causing sync/atomic operators to be implemented with the reverse operator, e.g. minus instead of plus. This should have been trivially caught by the test-suite, but wasn't, for three reasons. One, the sync_* test-suite effective-targets use hard-coded target tuples, and I haven't added cris and crisv32 there, doh (I see also m68k missing). I'll do that, with separate testing, perhaps later try to make the test general. Two, PR54266: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 and friends, as used in the test-suite effective-target cas_int, is of no use for targets implementing (some) atomics only through library functions. Three, mea culpa; I missed the following libstdc++ test-suite failures (4.7): FAIL: 20_util/shared_ptr/thread/default_weaktoshared.cc execution test FAIL: 20_util/shared_ptr/thread/mutex_weaktoshared.cc execution test FAIL: 21_strings/basic_string/pthread4.cc execution test FAIL: 23_containers/map/pthread6.cc execution test FAIL: 27_io/basic_ofstream/pthread2.cc execution test FAIL: 27_io/basic_ostringstream/pthread3.cc execution test FAIL: 30_threads/thread/members/4.cc execution test FAIL: 30_threads/thread/members/5.cc execution test FAIL: 30_threads/unique_lock/locking/2.cc execution test FAIL: tr1/2_general_utilities/shared_ptr/thread/default_weaktoshared.cc execution test FAIL: tr1/2_general_utilities/shared_ptr/thread/mutex_weaktoshared.cc execution test There *were* also some failures in the core gcc test-suite, like: FAIL: gcc.target/cris/torture/sync-mis-op-i-1ml.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) with gcc.log having: ccDrw3V5.ltrans0.o:(.text.startup+0x2a): undefined reference to `__atomic_fetch_nand_4' but that didn't make sense to me so I wrote it off as LTO weirdness (something wrong in LTO handling sync libfuncs with 4.7). It still doesn't really make sense to me, i.e. that the FAILs are now gone. I looked around and it seems only cris{v32,}-axis-linux-gnu is affected. Still, besides that target, for a 4.7/r189762 import and c/c++ testing, boot+regtest in progress for x86_64-linux and cross-test for cris-axis-elf. The test-case is spot-checked to pass for a target not implementing any atomics whatsoever, i.e. mmix-knuth-mmixware. Ok for trunk, assuming clean test-results? Maybe 4.7 too, it being somewhat trivial? gcc: PR middle-end/54261 * optabs.c (expand_atomic_fetch_op): Save and restore code when retrying after failed attempt. gcc/testsuite: PR middle-end/54261 * gcc.dg/torture/pr54261-1.c: New test. brgds, H-P --- /dev/null +++ gcc/testsuite/gcc.dg/torture/pr54261-1.c @@ -0,0 +1,42 @@ +/* { dg-do run } */ +/* { dg-additional-options "-DSYNC_FALLBACK" { target { ! cas_int } } } */ + +#ifdef SYNC_FALLBACK +/* The SYNC_FALLBACK code is just so we don't have to restrict this test + to any subset of targets. For targets with no atomics support at + all, the cas_int effective-target is false and the fallback provides + a PASS. Where the bug trigs (at the time this test-case was added), + cas_int is also false but the fallback isn't used. */ +__attribute__((__noinline__, __noclone__)) +int +# if __INT_MAX__ == 0x7fff + __sync_fetch_and_add_2 +# else + __sync_fetch_and_add_4 +# endif + (int *at, int val) +{ + int tmp = *at; + asm (""); + *at = tmp + val; + return tmp; +} +#endif + +__attribute__((__noinline__, __noclone__)) +void g (int *at, int val) +{ + asm (""); + __sync_fetch_and_add (at, val); +} + +int main(void) +{ + int x = 41; + int a = 1; + g (&x, a); + + if (x != 42) + __builtin_abort (); + __builtin_exit (0); +} (svn diff -x -U11 optabs.c used to show "code" being changed and then the conditional failure; nowhere else in expand_atomic_fetch_op is that done unless succeeded by unconditional success and return) Index: optabs.c =================================================================== --- gcc/optabs.c (revision 190398) +++ gcc/optabs.c (working copy) @@ -7818,22 +7818,23 @@ expand_atomic_fetch_op (rtx target, rtx result = expand_simple_binop (mode, code, result, val, target, true, OPTAB_LIB_WIDEN); return result; } } /* Try the __sync libcalls only if we can't do compare-and-swap inline. */ if (!can_compare_and_swap_p (mode, false)) { rtx libfunc; bool fixup = false; + enum rtx_code orig_code = code; libfunc = optab_libfunc (after ? optab.fetch_after : optab.fetch_before, mode); if (libfunc == NULL && (after || unused_result || optab.reverse_code != UNKNOWN)) { fixup = true; if (!after) code = optab.reverse_code; libfunc = optab_libfunc (after ? optab.fetch_before : optab.fetch_after, mode); @@ -7841,22 +7842,25 @@ expand_atomic_fetch_op (rtx target, rtx if (libfunc != NULL) { rtx addr = convert_memory_address (ptr_mode, XEXP (mem, 0)); result = emit_library_call_value (libfunc, NULL, LCT_NORMAL, mode, 2, addr, ptr_mode, val, mode); if (!unused_result && fixup) result = expand_simple_binop (mode, code, result, val, target, true, OPTAB_LIB_WIDEN); return result; } + + /* We need the original code for any further attempts. */ + code = orig_code; } /* If nothing else has succeeded, default to a compare and swap loop. */ if (can_compare_and_swap_p (mode, true)) { rtx insn; rtx t0 = gen_reg_rtx (mode), t1; start_sequence (); /* If the result is used, get a register for it. */