From patchwork Thu Dec 28 20:37:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 122872 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp3893745qgn; Thu, 28 Dec 2017 12:37:30 -0800 (PST) X-Google-Smtp-Source: ACJfBou1rUzJWOugk+Ss0r3hLgHoP3JKoQzXJBk2OJ+y6hDp7CGarPveAsDClQK+fxDoUgQooIW0 X-Received: by 10.84.225.132 with SMTP id u4mr32245377plj.120.1514493449989; Thu, 28 Dec 2017 12:37:29 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1514493449; cv=none; d=google.com; s=arc-20160816; b=Fkzc8wy9ww1EEJaJJtKFWT5PvNZ7Mcx820NwJFA1tpC+uimSDElN3BOtIOu5z0vOBc Sam1xotiwstcUyMv8J2620jHOCk6+CRT+QbBUuC51Zi3Ykdrb/lyiLtbX+g4zQLXcCZF V0gMkCCJDmknVLu1wodB1jMJyM1ay6GB3HnQGwzJWmxC8cyNlkAojTT+rwsb6WLbI9JK zhtK2UIbsE1EmnTIpoAMDVRw3maMIfDQI2C6rqKEyIarSNO+Dj7/Ej0H7lzu1KReFmTF SJHyT3TLFVGo/yED+Gmq986otrkWCWcYo5sGPpggZDH4sOO+NbNWY3/+96PDsYM48rnG qVqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:mail-followup-to:to:from:delivered-to:sender:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :mailing-list:dkim-signature:domainkey-signature :arc-authentication-results; bh=hCgv0/yDwinO4alh/rZRWd7TE81bGBWUL5QowUYezwA=; b=wrfBs7O+1dZlu0j5AA/H6yTi04RWa+JKV+nCqnhEOug0+rQQEGojRr4lD/MszTsmcO JMKI+xhnxlQjxYbMphz0/nls1Si+gAW/s4/UDbR37lOhiJEI8f+9pJl4avwCftgkovTa 2PaRrUG60swAxgDSDLBeLSZIKXUoVIg18liILFN2XR6SZ+9j7hQMEltSOhNkziGb8kaR SPzxGGiXFu6TcCy13Vm4VB/1ntCnjiZvPU2gawE7WCg3krqLe/uySWbRXreiUmhG5ClT JrRzXrQ2XOZ3+Xz9n9+Nv92Fg47xtgVAwfu/YWiMWG6cVjI5lC1Bj4EBkV7etPpXVZu3 G+eg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=trnolnC3; spf=pass (google.com: domain of gcc-patches-return-469867-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-469867-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id l24si18050291pgn.528.2017.12.28.12.37.29 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Dec 2017 12:37:29 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-469867-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=trnolnC3; spf=pass (google.com: domain of gcc-patches-return-469867-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-469867-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=HYJkfteY7ayAZHGN 6cmLjwjZmKmKiFm6ve1vKF/yCHSc19SejdkyKuwJTzKvw1HRQ5SYaRmddbxbTuVd AV4azh479TmIdXcDxBY5A33rxoRBFP4wKI7oRhJpBzK6QqREmRODEHTuPAGkrkJ3 NgJiQYh9ZHSq4EeoEkVvlnmSmNU= 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:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=aOYxCjvtXwH9QkLbQ7P3Bo HiCe8=; b=trnolnC3uy+U6d/IFYLX86HZEqK7udk+Q8F60NFjHDdC9w5fUcDNA/ qdeuhLIui2Nk8dGQnjhTv5guwV2m7Pctyuue80iAN6riyb5KHEc1u40PhFVvWvS+ TGdFR4s56Bl7fFROc/XevUFujrly9qxBi0o8gBDGcBPuVCGlLQQio= Received: (qmail 61788 invoked by alias); 28 Dec 2017 20:37:19 -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 61772 invoked by uid 89); 28 Dec 2017 20:37:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=tended, disregard, throwing X-HELO: mail-wr0-f171.google.com Received: from mail-wr0-f171.google.com (HELO mail-wr0-f171.google.com) (209.85.128.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 28 Dec 2017 20:37:17 +0000 Received: by mail-wr0-f171.google.com with SMTP id f8so29365731wre.4 for ; Thu, 28 Dec 2017 12:37:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=hCgv0/yDwinO4alh/rZRWd7TE81bGBWUL5QowUYezwA=; b=l5UaJdEsJDChyUf1T1LdhKakFQgrhtG4rJiB4wfh+KSg540i/X8vNgLHLBCGdOd+8c fZVxs36pn68uS+50P+9Krvms9fXI1ylAcyKVUkBZ48gZxk13zFhO8ItSJFxVcid5Uij7 vfukKG8mp/Gys4CgNcAat0bOwdG+iXWI/4HXJgXAd0v/aE1oqBAoQr8pwZvawYlhG/Kl f1yBtRic0UwrpOy3tVivWKIk9baVBp3nPWTQPHhQRh7fxyBSTc1iiqzPUflDQaRt8XPd /e0V+al1e8CiBfzEckwP+58Ed/jb1WK360FRG6XO+SS87RhBoFufoHbM8JdfUMOUCLJj IpHw== X-Gm-Message-State: AKGB3mK8p8ViNHXypL06zzcXn4FsD1JK79JoNwg+bC1BGPR2PFKpD7qp P8GzN9/SiKuhTZSTjIyruiT5P+h6+e0= X-Received: by 10.223.139.73 with SMTP id v9mr33061913wra.77.1514493435068; Thu, 28 Dec 2017 12:37:15 -0800 (PST) Received: from localhost ([95.145.138.186]) by smtp.gmail.com with ESMTPSA id m134sm19698224wmg.6.2017.12.28.12.37.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 28 Dec 2017 12:37:14 -0800 (PST) From: Richard Sandiford To: Andreas Schwab Mail-Followup-To: Andreas Schwab , gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org Subject: RFA: Fix REG_ARGS_SIZE handling when pushing TLS addresses References: <871sltvm7r.fsf@linaro.org> <87h8upn5nm.fsf@linaro.org> <877etdhjl5.fsf@linaro.org> Date: Thu, 28 Dec 2017 20:37:13 +0000 In-Reply-To: (Andreas Schwab's message of "Sun, 24 Dec 2017 13:49:25 +0100") Message-ID: <87h8saio7a.fsf_-_@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Andreas Schwab writes: > On Dez 23 2017, Richard Sandiford wrote: >> gcc/ >> * expr.c (fixup_args_size_notes): Check that any existing >> REG_ARGS_SIZE notes are correct, and don't try to re-add them. >> (emit_single_push_insn_1): Move stack_pointer_delta adjustment to... >> (emit_single_push_insn): ...here. > > Successfully regtested on m68k-linux. Thanks. Now also tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu (not that that will give mucn coverage). Also tested with a before-and-after comparison of testsuite output for a range of targets. OK to install? Richard The new assert in add_args_size_note triggered for gcc.dg/tls/opt-3.c and other on m68k. This looks like a pre-existing bug: if we pushed a value that needs a call to something like __tls_get_addr, we ended up with two different REG_ARGS_SIZE notes on the same instruction. It seems to be OK for emit_single_push_insn to push something that needs a call to __tls_get_addr: /* We have to allow non-call_pop patterns for the case of emit_single_push_insn of a TLS address. */ if (GET_CODE (pat) != PARALLEL) return 0; so I think the bug is in the way this is handled rather than the fact that it occurs at all. If we're pushing a value X that needs a call C to calculate, we'll add REG_ARGS_SIZE notes to the pushes and pops for C as part of the call sequence. Then emit_single_push_insn calls fixup_args_size_notes on the whole push sequence (the calculation of X, including C, and the push of X itself). This is where the double notes came from. But emit_single_push_insn_1 adjusted stack_pointer_delta *before* the push, so the notes added for C were relative to the situation after the future push of X rather than before it. Presumably this didn't matter in practice because the note added second tended to trump the note added first. But code is allowed to walk REG_NOTES without having to disregard secondary notes. 2017-12-23 Richard Sandiford gcc/ * expr.c (fixup_args_size_notes): Check that any existing REG_ARGS_SIZE notes are correct, and don't try to re-add them. (emit_single_push_insn_1): Move stack_pointer_delta adjustment to... (emit_single_push_insn): ...here. Index: gcc/expr.c =================================================================== --- gcc/expr.c 2017-12-23 09:29:20.226338285 +0000 +++ gcc/expr.c 2017-12-23 09:29:45.783339673 +0000 @@ -4089,6 +4089,14 @@ fixup_args_size_notes (rtx_insn *prev, r if (!NONDEBUG_INSN_P (insn)) continue; + /* We might have existing REG_ARGS_SIZE notes, e.g. when pushing + a call argument containing a TLS address that itself requires + a call to __tls_get_addr. The handling of stack_pointer_delta + in emit_single_push_insn is supposed to ensure that any such + notes are already correct. */ + rtx note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX); + gcc_assert (!note || known_eq (args_size, get_args_size (note))); + poly_int64 this_delta = find_args_size_adjust (insn); if (known_eq (this_delta, 0)) { @@ -4102,7 +4110,8 @@ fixup_args_size_notes (rtx_insn *prev, r if (known_eq (this_delta, HOST_WIDE_INT_MIN)) saw_unknown = true; - add_args_size_note (insn, args_size); + if (!note) + add_args_size_note (insn, args_size); if (STACK_GROWS_DOWNWARD) this_delta = -poly_uint64 (this_delta); @@ -4126,7 +4135,6 @@ emit_single_push_insn_1 (machine_mode mo rtx dest; enum insn_code icode; - stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode)); /* If there is push pattern, use it. Otherwise try old way of throwing MEM representing push operation to move expander. */ icode = optab_handler (push_optab, mode); @@ -4213,6 +4221,14 @@ emit_single_push_insn (machine_mode mode emit_single_push_insn_1 (mode, x, type); + /* Adjust stack_pointer_delta to describe the situation after the push + we just performed. Note that we must do this after the push rather + than before the push in case calculating X needs pushes and pops of + its own (e.g. if calling __tls_get_addr). The REG_ARGS_SIZE notes + for such pushes and pops must not include the effect of the future + push of X. */ + stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode)); + last = get_last_insn (); /* Notice the common case where we emitted exactly one insn. */