From patchwork Fri Mar 15 18:16:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 228135 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 129562C00B7 for ; Sat, 16 Mar 2013 05:17:21 +1100 (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=1363976243; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Date:From:To:CC:Subject:Message-ID:MIME-Version: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=cyJie6t 53Vb40H6RMpCF17b2a8M=; b=yY48XYlf1mMtHQa07JTxKsyN2y9ngvyd37Yde/j 3ADi78cMxsFx3DscmRajNIleoXpRAOk1evV7F0r1t0u+kHGoOAbST6uRHzFBqORa mkEuC8fHhTN0E+Z+1P7KL3oLQ9HKexD4obBZzp27QKtX79SkHmBDQsoEzNx6Rk85 p5Vk= 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:Date:From:To:CC:Subject:Message-ID:MIME-Version:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=uQsVbHhGqOeibff//rla/inZ8rZiwcLtDWntKq5bO3h+jnf4BWXgHdhaZ0rPqt ZRtetVW59hd7N3UI3TBdd5dlT3bPoPK67ty4CRJbMgG4CbW4N5TDfNKz6DmITw7k ekmJGcZ8pIufz2FJL03wDZDvkWgf44JPmBcTS62qTKWec=; Received: (qmail 25865 invoked by alias); 15 Mar 2013 18:17:11 -0000 Received: (qmail 25852 invoked by uid 22791); 15 Mar 2013 18:17:08 -0000 X-SWARE-Spam-Status: No, hits=-3.5 required=5.0 tests=AWL, BAYES_00, FROM_12LTRDOM, KHOP_RCVD_UNTRUST, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 15 Mar 2013 18:16:59 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UGZBZ-00061S-Mu from Julian_Brown@mentor.com ; Fri, 15 Mar 2013 11:16:57 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 15 Mar 2013 11:16:57 -0700 Received: from octopus (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Fri, 15 Mar 2013 18:16:55 +0000 Date: Fri, 15 Mar 2013 18:16:48 +0000 From: Julian Brown To: CC: , Marcus Shawcroft , Ramana Radhakrishnan Subject: [PATCH, ARM] ARM Linux kernel-assisted atomic operation helpers vs. libcall argument promotion Message-ID: <20130315181648.60367d9c@octopus> MIME-Version: 1.0 X-IsSubscribed: yes 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 Hi, At present, the libcall helpers implementing atomic operations (__sync_val_compare_and_swap_X) for char and short types suffer from a type mismatch. This is leading to test failures, i.e.: FAIL: gcc.dg/atomic-compare-exchange-1.c execution test FAIL: gcc.dg/atomic-compare-exchange-2.c execution test On investigation, these tests pass if the values used in the tests are tweaked so that they are in the range representable by both signed and unsigned chars, i.e. 0 to 127, rather than ~0. The failures are happening because libcall expansion is sign-extending sub-word-size arguments (e.g. EXPECTED, DESIRED in optabs.c:expand_atomic_compare_and_swap), but the functions implementing the operations are written to take unsigned arguments, zero-extended, and the unexpected out-of-range values cause them to fail. The sign-extension happens because in calls.c:emit_library_call_value_1 we have: mode = promote_function_mode (NULL_TREE, mode, &unsigned_p, NULL_TREE, 0); argvec[count].mode = mode; argvec[count].value = convert_modes (mode, GET_MODE (val), val, unsigned_p); argvec[count].reg = targetm.calls.function_arg (args_so_far, mode, NULL_TREE, true); This calls back into arm.c:arm_promote_function_mode, which promotes less-than-four-byte integral values to SImode, but never modifies the PUNSIGNEDP argument. So, such values always get sign extended when being passed to libcalls. The simplest fix for this (since libcalls don't have proper tree types to inspect for the actual argument types) is just to define the linux-atomic.c functions to use signed char/short instead of unsigned char/unsigned short, approximately reversing the change in this earlier patch: http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00492.html A slight change is also required to the __sync_val_compare_and_swap_* implementation in order to treat the signed OLDVAL argument correctly (I believe the other macros are OK). Tested cross to ARM Linux, default & thumb multilibs. The above-mentioned tests change from FAIL to PASS. OK to apply? Thanks, Julian ChangeLog libgcc/ * config/arm/linux-atomic.c (SUBWORD_SYNC_OP, SUBWORD_VAL_CAS) (SUBWORD_TEST_AND_SET): Use signed char/short types instead of unsigned char/unsigned short. (__sync_val_compare_and_swap_{1,2}): Handle signed argument. Index: libgcc/config/arm/linux-atomic.c =================================================================== --- libgcc/config/arm/linux-atomic.c (revision 196648) +++ libgcc/config/arm/linux-atomic.c (working copy) @@ -97,19 +97,19 @@ FETCH_AND_OP_WORD (nand, ~, &) return (RETURN & mask) >> shift; \ } -SUBWORD_SYNC_OP (add, , +, unsigned short, 2, oldval) -SUBWORD_SYNC_OP (sub, , -, unsigned short, 2, oldval) -SUBWORD_SYNC_OP (or, , |, unsigned short, 2, oldval) -SUBWORD_SYNC_OP (and, , &, unsigned short, 2, oldval) -SUBWORD_SYNC_OP (xor, , ^, unsigned short, 2, oldval) -SUBWORD_SYNC_OP (nand, ~, &, unsigned short, 2, oldval) - -SUBWORD_SYNC_OP (add, , +, unsigned char, 1, oldval) -SUBWORD_SYNC_OP (sub, , -, unsigned char, 1, oldval) -SUBWORD_SYNC_OP (or, , |, unsigned char, 1, oldval) -SUBWORD_SYNC_OP (and, , &, unsigned char, 1, oldval) -SUBWORD_SYNC_OP (xor, , ^, unsigned char, 1, oldval) -SUBWORD_SYNC_OP (nand, ~, &, unsigned char, 1, oldval) +SUBWORD_SYNC_OP (add, , +, short, 2, oldval) +SUBWORD_SYNC_OP (sub, , -, short, 2, oldval) +SUBWORD_SYNC_OP (or, , |, short, 2, oldval) +SUBWORD_SYNC_OP (and, , &, short, 2, oldval) +SUBWORD_SYNC_OP (xor, , ^, short, 2, oldval) +SUBWORD_SYNC_OP (nand, ~, &, short, 2, oldval) + +SUBWORD_SYNC_OP (add, , +, signed char, 1, oldval) +SUBWORD_SYNC_OP (sub, , -, signed char, 1, oldval) +SUBWORD_SYNC_OP (or, , |, signed char, 1, oldval) +SUBWORD_SYNC_OP (and, , &, signed char, 1, oldval) +SUBWORD_SYNC_OP (xor, , ^, signed char, 1, oldval) +SUBWORD_SYNC_OP (nand, ~, &, signed char, 1, oldval) #define OP_AND_FETCH_WORD(OP, PFX_OP, INF_OP) \ int HIDDEN \ @@ -132,19 +132,19 @@ OP_AND_FETCH_WORD (and, , &) OP_AND_FETCH_WORD (xor, , ^) OP_AND_FETCH_WORD (nand, ~, &) -SUBWORD_SYNC_OP (add, , +, unsigned short, 2, newval) -SUBWORD_SYNC_OP (sub, , -, unsigned short, 2, newval) -SUBWORD_SYNC_OP (or, , |, unsigned short, 2, newval) -SUBWORD_SYNC_OP (and, , &, unsigned short, 2, newval) -SUBWORD_SYNC_OP (xor, , ^, unsigned short, 2, newval) -SUBWORD_SYNC_OP (nand, ~, &, unsigned short, 2, newval) - -SUBWORD_SYNC_OP (add, , +, unsigned char, 1, newval) -SUBWORD_SYNC_OP (sub, , -, unsigned char, 1, newval) -SUBWORD_SYNC_OP (or, , |, unsigned char, 1, newval) -SUBWORD_SYNC_OP (and, , &, unsigned char, 1, newval) -SUBWORD_SYNC_OP (xor, , ^, unsigned char, 1, newval) -SUBWORD_SYNC_OP (nand, ~, &, unsigned char, 1, newval) +SUBWORD_SYNC_OP (add, , +, short, 2, newval) +SUBWORD_SYNC_OP (sub, , -, short, 2, newval) +SUBWORD_SYNC_OP (or, , |, short, 2, newval) +SUBWORD_SYNC_OP (and, , &, short, 2, newval) +SUBWORD_SYNC_OP (xor, , ^, short, 2, newval) +SUBWORD_SYNC_OP (nand, ~, &, short, 2, newval) + +SUBWORD_SYNC_OP (add, , +, signed char, 1, newval) +SUBWORD_SYNC_OP (sub, , -, signed char, 1, newval) +SUBWORD_SYNC_OP (or, , |, signed char, 1, newval) +SUBWORD_SYNC_OP (and, , &, signed char, 1, newval) +SUBWORD_SYNC_OP (xor, , ^, signed char, 1, newval) +SUBWORD_SYNC_OP (nand, ~, &, signed char, 1, newval) int HIDDEN __sync_val_compare_and_swap_4 (int *ptr, int oldval, int newval) @@ -181,7 +181,7 @@ __sync_val_compare_and_swap_4 (int *ptr, actual_oldval = *wordptr; \ \ if (__builtin_expect (((actual_oldval & mask) >> shift) != \ - (unsigned int) oldval, 0)) \ + ((unsigned int) oldval & MASK_##WIDTH), 0)) \ return (actual_oldval & mask) >> shift; \ \ actual_newval = (actual_oldval & ~mask) \ @@ -195,8 +195,8 @@ __sync_val_compare_and_swap_4 (int *ptr, } \ } -SUBWORD_VAL_CAS (unsigned short, 2) -SUBWORD_VAL_CAS (unsigned char, 1) +SUBWORD_VAL_CAS (short, 2) +SUBWORD_VAL_CAS (signed char, 1) typedef unsigned char bool; @@ -217,8 +217,8 @@ __sync_bool_compare_and_swap_4 (int *ptr return (oldval == actual_oldval); \ } -SUBWORD_BOOL_CAS (unsigned short, 2) -SUBWORD_BOOL_CAS (unsigned char, 1) +SUBWORD_BOOL_CAS (short, 2) +SUBWORD_BOOL_CAS (signed char, 1) void HIDDEN __sync_synchronize (void) @@ -260,8 +260,8 @@ __sync_lock_test_and_set_4 (int *ptr, in return (oldval & mask) >> shift; \ } -SUBWORD_TEST_AND_SET (unsigned short, 2) -SUBWORD_TEST_AND_SET (unsigned char, 1) +SUBWORD_TEST_AND_SET (short, 2) +SUBWORD_TEST_AND_SET (signed char, 1) #define SYNC_LOCK_RELEASE(TYPE, WIDTH) \ void HIDDEN \