From patchwork Mon Sep 2 20:35:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 1156722 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-508187-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=de.ibm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="qvjOC1kH"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46Mhfz6dPRz9sDB for ; Tue, 3 Sep 2019 06:35:37 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:date:from:mime-version:content-type :content-transfer-encoding:message-id; q=dns; s=default; b=IHKI6 Xp4CdF1zR7B9esliLgMz60OZ7sp4LliWby9mfRQKC+nbiF8uajXeNPTl1cuXER5p 4aRWqlFYq3P1OCXezsjzO6Fv2muj8zSzYxKBMCcu0rw961dSqJlSXAuSeLuCp5ai OK2FE3BPLkwoWv8muB14l3J7CtiHX3UPuC/TUk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:date:from:mime-version:content-type :content-transfer-encoding:message-id; s=default; bh=oBdqGTfbc4b 6fl7y7zekjrjDcR8=; b=qvjOC1kHIaYsHadoQ9wQeemQQh8NOQNIwmo9bnvjRHi KmGxIRk4n5UCx/NvyDpgPLr/H+jiHyjbjhF06VfU9OQO8KMinZxdT4v6OlmBEbtU 09dHX9ipToAEPTixwUqvtRm/WYis09zJYE57+/cv1rTytn+BsPej3TCbNU/eOTgg = Received: (qmail 82097 invoked by alias); 2 Sep 2019 20:35:29 -0000 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 Received: (qmail 82089 invoked by uid 89); 2 Sep 2019 20:35:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.1 spammy=bits_per_unit, BITS_PER_UNIT, Respect X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Sep 2019 20:35:28 +0000 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x82KVu1Q030769 for ; Mon, 2 Sep 2019 16:35:25 -0400 Received: from e06smtp03.uk.ibm.com (e06smtp03.uk.ibm.com [195.75.94.99]) by mx0b-001b2d01.pphosted.com with ESMTP id 2us832339r-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 02 Sep 2019 16:35:25 -0400 Received: from localhost by e06smtp03.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 2 Sep 2019 21:35:24 +0100 Received: from b06cxnps4076.portsmouth.uk.ibm.com (9.149.109.198) by e06smtp03.uk.ibm.com (192.168.101.133) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Mon, 2 Sep 2019 21:35:22 +0100 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x82KZL8H40763540 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 2 Sep 2019 20:35:21 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5D10542049 for ; Mon, 2 Sep 2019 20:35:21 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 439EE4203F for ; Mon, 2 Sep 2019 20:35:21 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.145.82.107]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP for ; Mon, 2 Sep 2019 20:35:21 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id B4025D802F0; Mon, 2 Sep 2019 22:35:20 +0200 (CEST) Subject: [PATCH] Use type alignment in get_builtin_sync_mem To: gcc-patches@gcc.gnu.org Date: Mon, 2 Sep 2019 22:35:20 +0200 (CEST) From: "Ulrich Weigand" MIME-Version: 1.0 x-cbid: 19090220-0012-0000-0000-00000345D1CE X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19090220-0013-0000-0000-000021801D88 Message-Id: <20190902203520.B4025D802F0@oc3748833570.ibm.com> Hello, on s390x the 128-bit integer type is only aligned to 8 bytes by default, but when lock-free atomic operations can only be performed on objects aligned to 16 bytes. However, we've noticed that GCC sometimes falls back to library calls *even if* the object is actually 16 byte aligned, and GCC could easily know this from type alignment information. However, it turns out that get_builtin_sync_mem *ignores* type alignment data, and only looks at *mode* alignment and pointer alignment data. This is a problem since mode alignment doesn't know about user-provided alignment attributes, and while pointer alignment does sometimes know about those, this usually only happens with optimization on and also if the optimizers can trace pointer assignments to the final object. One important use case where the latter does not happen is with the C++ atomic classes, because here the object is accessed via the "this" pointer. Pointer alignment tracking at this point never knows the final object, and thus we always get a library call *even though* libstdc++ has actually marked the class type with the correct alignment atttribute. Now one question might be, why does get_pointer_alignment not take type alignment into account by itself? This appears to be deliberate to avoid considering numeric pointer values to be aligned when they are not for target-specific reasons (e.g. the low bit that indicates Thumb on ARM). However, this is not an issue in get_builtin_sync_mem, where we are actually interested in the alignment of the MEM we're just about to generate, so it should be fine to check type alignment here. This patch does just that, fixing the issue we've been seeing. Tested on s390x-ibm-linux. OK for mainline? Bye, Ulrich ChangeLog: * builtins.c (get_builtin_sync_mem): Respect type alignment. testsuite/ChangeLog: * gcc.target/s390/md/atomic_exchange-1.c: Do not use -latomic. (aligned_int128): New data type. (ae_128_0): Use it instead of __int128 to ensure proper alignment for atomic accesses. (ae_128_1): Likewise. (g128): Likewise. (main): Likewise. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c (revision 274142) +++ gcc/builtins.c (working copy) @@ -6001,9 +6001,16 @@ mem = validize_mem (mem); - /* The alignment needs to be at least according to that of the mode. */ - set_mem_align (mem, MAX (GET_MODE_ALIGNMENT (mode), - get_pointer_alignment (loc))); + /* The alignment needs to be at least according to that of the mode. + Also respect alignment requirements of the type, and alignment + info that may be deduced from the expression itself. */ + unsigned int align = GET_MODE_ALIGNMENT (mode); + if (POINTER_TYPE_P (TREE_TYPE (loc))) + { + unsigned int talign = min_align_of_type (TREE_TYPE (TREE_TYPE (loc))); + align = MAX (align, talign * BITS_PER_UNIT); + } + set_mem_align (mem, MAX (align, get_pointer_alignment (loc))); set_mem_alias_set (mem, ALIAS_SET_MEMORY_BARRIER); MEM_VOLATILE_P (mem) = 1; Index: gcc/testsuite/gcc.target/s390/md/atomic_exchange-1.c =================================================================== --- gcc/testsuite/gcc.target/s390/md/atomic_exchange-1.c (revision 274142) +++ gcc/testsuite/gcc.target/s390/md/atomic_exchange-1.c (working copy) @@ -1,7 +1,7 @@ /* Machine description pattern tests. */ /* { dg-do compile } */ -/* { dg-options "-lpthread -latomic" } */ +/* { dg-options "-lpthread" } */ /* { dg-do run { target { s390_useable_hw } } } */ /**/ @@ -119,19 +119,21 @@ /**/ #ifdef __s390x__ +typedef __int128 __attribute__((aligned(16))) aligned_int128; + __int128 -ae_128_0 (__int128 *lock) +ae_128_0 (aligned_int128 *lock) { return __atomic_exchange_n (lock, 0, 2); } __int128 -ae_128_1 (__int128 *lock) +ae_128_1 (aligned_int128 *lock) { return __atomic_exchange_n (lock, 1, 2); } -__int128 g128; +aligned_int128 g128; __int128 ae_128_g_0 (void) @@ -274,7 +276,7 @@ #ifdef __s390x__ { - __int128 lock; + aligned_int128 lock; __int128 rval; lock = oval;