From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 652 invoked by alias); 15 Mar 2004 23:57:46 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 644 invoked from network); 15 Mar 2004 23:57:42 -0000 Received: from unknown (HELO av.mvista.com) (12.44.186.158) by sources.redhat.com with SMTP; 15 Mar 2004 23:57:42 -0000 Received: from mvista.com (av [127.0.0.1]) by av.mvista.com (8.9.3/8.9.3) with ESMTP id PAA13171 for ; Mon, 15 Mar 2004 15:57:41 -0800 Message-ID: <405642CE.F6DB290F@mvista.com> Date: Mon, 15 Mar 2004 23:57:00 -0000 From: Michael Eager Organization: MontaVista Software, Inc. MIME-Version: 1.0 To: gcc Subject: locking problem with mips atomicity Content-Type: multipart/mixed; boundary="------------8A511AE88E03D9AA2BE4C0AB" X-SW-Source: 2004-03/txt/msg00755.txt.bz2 This is a multi-part message in MIME format. --------------8A511AE88E03D9AA2BE4C0AB Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 2254 The MIPS architecture spec says that there should not be a load, store or prefetch between issuing a LL instruction and a following SC instruction. (MIPS32 Arch, Vol 2, rev 0.95, SC instruction description, p 186) The macros in libstdc++-v3/config/cpu/mips/atomicity.h (and similar code in glibc) could generate a load between the LL and SC if they were inlined. The comments in the macros still mentione inline, but "inline" was removed in the most recent mod to this header (2/27/04). The functions generated without inline do not appear to have this problem, but previous versions of gcc inlined the code do. (Offhand, I think removing inline for locking code will have an adverse affect on performance.) This program (adapted from mcount.c in glibc) shows the problem: mcount.c: typedef int _Atomic_word; #include "atomicity.h" using namespace __gnu_cxx; struct gmonparam { _Atomic_word state; }; extern struct gmonparam _gmonparam ; static void __attribute__ ((__used__)) __mcount () { register struct gmonparam *p; p = &_gmonparam; if (! __exchange_and_add(&p->state, 1)) return; p->state = 0; } The code generated for exchange_and_add is (lwc0 == LL, swc0 == SC): 10: 8f830000 lw v1,0(gp) 14: 00000000 nop 18: c0630000 lwc0 $3,0(v1) 1c: 00432021 addu a0,v0,v1 20: 8f810000 lw at,0(gp) <<<<<< BAD 24: 00000000 nop 28: e0240000 swc0 $4,0(at) 2c: 1080fff8 beqz a0,10 <_Z8__mcountv+0x10> 30: 00000000 nop 34: 10600004 beqz v1,48 <_Z8__mcountv+0x48> The attached patch loads the address of the location being modified into a reg before the LL, and references it explicitly, so that the following is generated instead: 10: 8f840000 lw a0,0(gp) 14: c0830000 lwc0 $3,0(a0) 18: 00432821 addu a1,v0,v1 1c: e0850000 swc0 $5,0(a0) 20: 10a0fffb beqz a1,10 <_Z8__mcountv+0x10> 24: 00000000 nop 28: 10600004 beqz v1,3c <_Z8__mcountv+0x3c> -- Michael Eager eager@mvista.com 408-328-8426 MontaVista Software, Inc. 1237 E. Arques Ave., Sunnyvale, CA 94085 --------------8A511AE88E03D9AA2BE4C0AB Content-Type: text/plain; charset=us-ascii; name="FSF-gcc.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="FSF-gcc.patch" Content-length: 1921 Index: libstdc++-v3/config/cpu/mips/atomicity.h =================================================================== RCS file: /cvs/gcc/gcc/libstdc++-v3/config/cpu/mips/atomicity.h,v retrieving revision 1.9 diff -u -r1.9 atomicity.h --- libstdc++-v3/config/cpu/mips/atomicity.h 27 Feb 2004 00:49:48 -0000 1.9 +++ libstdc++-v3/config/cpu/mips/atomicity.h 15 Mar 2004 23:55:31 -0000 @@ -35,7 +35,7 @@ __attribute__ ((__unused__)) __exchange_and_add(volatile _Atomic_word* __mem, int __val) { - _Atomic_word __result, __tmp; + _Atomic_word __result, __tmp, temp; __asm__ __volatile__ ("/* Inline exchange & add */\n\t" @@ -44,13 +44,14 @@ #if _MIPS_SIM == _ABIO32 ".set mips2\n\t" #endif - "ll %0,%3\n\t" - "addu %1,%4,%0\n\t" - "sc %1,%2\n\t" + "la %3,%2\n\t" + "ll %0,0(%3)\n\t" + "addu %1,%5,%0\n\t" + "sc %1,0(%3)\n\t" ".set pop\n\t" "beqz %1,1b\n\t" "/* End exchange & add */" - : "=&r"(__result), "=&r"(__tmp), "=m"(*__mem) + : "=&r"(__result), "=&r"(__tmp), "=m"(*__mem), "=r" (temp) : "m" (*__mem), "r"(__val)); return __result; @@ -60,7 +61,7 @@ __attribute__ ((__unused__)) __atomic_add(volatile _Atomic_word* __mem, int __val) { - _Atomic_word __result; + _Atomic_word __result, temp; __asm__ __volatile__ ("/* Inline atomic add */\n\t" @@ -69,13 +70,14 @@ #if _MIPS_SIM == _ABIO32 ".set mips2\n\t" #endif - "ll %0,%2\n\t" - "addu %0,%3,%0\n\t" - "sc %0,%1\n\t" + "la %2,%1\n\t" + "ll %0,0(%2)\n\t" + "addu %0,%4,%0\n\t" + "sc %0,0(%2)\n\t" ".set pop\n\t" "beqz %0,1b\n\t" "/* End atomic add */" - : "=&r"(__result), "=m"(*__mem) + : "=&r"(__result), "=m"(*__mem), "=r" (temp) : "m" (*__mem), "r"(__val)); } } // namespace __gnu_cxx --------------8A511AE88E03D9AA2BE4C0AB--