From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4897 invoked by alias); 3 Sep 2019 14:01:46 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4889 invoked by uid 89); 3 Sep 2019 14:01:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT autolearn=ham version=3.3.1 spammy=satisfy X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Sep 2019 14:01:44 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AC2A2337; Tue, 3 Sep 2019 07:01:42 -0700 (PDT) Received: from [10.2.206.47] (e120808-lin.cambridge.arm.com [10.2.206.47]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3B2053F246; Tue, 3 Sep 2019 07:01:42 -0700 (PDT) Subject: Re: [PATCH 02/25] Propagate address spaces to builtins. To: "ams@codesourcery.com" , "gcc-patches@gcc.gnu.org" , richard.henderson@linaro.org References: From: Kyrill Tkachov Message-ID: Date: Tue, 03 Sep 2019 14:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2019-09/txt/msg00118.txt.bz2 Hi all, On 9/5/18 12:48 PM, ams@codesourcery.com wrote: > > At present, pointers passed to builtin functions, including atomic > operators, > are stripped of their address space properties.  This doesn't seem to be > deliberate, it just omits to copy them. > > Not only that, but it forces pointer sizes to Pmode, which isn't > appropriate > for all address spaces. > > This patch attempts to correct both issues.  It works for GCN atomics and > GCN OpenACC gang-private variables. > > 2018-09-05  Andrew Stubbs  >             Julian Brown  > >         gcc/ >         * builtins.c (get_builtin_sync_mem): Handle address spaces. Sorry for responding to this so late. I'm testing a rebased version of Richard's OOL atomic patches [1] and am hitting an ICE building the -mabi=ilp32 libgfortran multilib for aarch64-none-elf: 0x7284db emit_library_call_value_1(int, rtx_def*, rtx_def*, libcall_type, machine_mode, int, std::pair*)         $SRC/gcc/calls.c:4915 0x1037817 emit_library_call_value(rtx_def*, rtx_def*, libcall_type, machine_mode, rtx_def*, machine_mode, rtx_def*, machine_mode, rtx_def*, machine_mode)         $SRC/gcc/rtl.h:4240 0x1037817 aarch64_expand_compare_and_swap(rtx_def**)         $SRC/gcc/config/aarch64/aarch64.c:16981 0x1353a43 gen_atomic_compare_and_swapsi(rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*)         $SRC/gcc/config/aarch64/atomics.md:34 0xb1f9f1 insn_gen_fn::operator()(rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*) const         $SRC/gcc/recog.h:324 0xb1f9f1 maybe_gen_insn(insn_code, unsigned int, expand_operand*)         $SRC/gcc/optabs.c:7443 0xb1fa78 maybe_expand_insn(insn_code, unsigned int, expand_operand*)         $SRC/gcc/optabs.c:7459 0xb21024 expand_atomic_compare_and_swap(rtx_def**, rtx_def**, rtx_def*, rtx_def*, rtx_def*, bool, memmodel, memmodel)         $SRC/gcc/optabs.c:6448 0x709bd3 expand_builtin_atomic_compare_exchange         $SRC/gcc/builtins.c:6379 0x71a4e9 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)         $SRC/gcc/builtins.c:8147 0x88b746 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)         $SRC/gcc/expr.c:11052 0x88cce6 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)         $SRC/gcc/expr.c:8289 0x74cb47 expand_expr         $SRC/gcc/expr.h:281 0x74cb47 expand_call_stmt         $SRC/gcc/cfgexpand.c:2731 0x74cb47 expand_gimple_stmt_1         $SRC/gcc/cfgexpand.c:3710 0x74cb47 expand_gimple_stmt         $SRC/gcc/cfgexpand.c:3875 0x75439b expand_gimple_basic_block         $SRC/gcc/cfgexpand.c:5915 0x7563ab execute         $SRC/gcc/cfgexpand.c:6538 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See for instructions. A MEM rtx now uses a DImode address where for ILP32 we expect SImode. This looks to be because.... [1] https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00062.html > --- >  gcc/builtins.c | 13 ++++++++++--- >  1 file changed, 10 insertions(+), 3 deletions(-) > 0002-Propagate-address-spaces-to-builtins.patch diff --git a/gcc/builtins.c b/gcc/builtins.c index 58ea747..361361c 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5781,14 +5781,21 @@ static rtx get_builtin_sync_mem (tree loc, machine_mode mode) { rtx addr, mem; + int addr_space = TYPE_ADDR_SPACE (POINTER_TYPE_P (TREE_TYPE (loc)) + ? TREE_TYPE (TREE_TYPE (loc)) + : TREE_TYPE (loc)); + scalar_int_mode addr_mode = targetm.addr_space.address_mode (addr_space); ... This now returns Pmode (the default for the hook) for aarch64 ILP32, which is always DImode. - addr = expand_expr (loc, NULL_RTX, ptr_mode, EXPAND_SUM); Before this patch we used ptr_mode, which does the right thing for AArch64 ILP32. Do you think we should just be implementing targetm.addr_space.address_mode for AArch64 to return SImode for ILP32? Thanks, Kyrill - addr = convert_memory_address (Pmode, addr); + addr = expand_expr (loc, NULL_RTX, addr_mode, EXPAND_SUM); /* Note that we explicitly do not want any alias information for this memory, so that we kill all other live memories. Otherwise we don't satisfy the full barrier semantics of the intrinsic. */ - mem = validize_mem (gen_rtx_MEM (mode, addr)); + mem = gen_rtx_MEM (mode, addr); + + set_mem_addr_space (mem, addr_space); + + 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),