From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91608 invoked by alias); 2 Jan 2018 19:07:12 -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 91598 invoked by uid 89); 2 Jan 2018 19:07:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=trump, HTo:U*schwab, H*M:2dc1 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 02 Jan 2018 19:07:11 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED67AC047B8B; Tue, 2 Jan 2018 19:07:09 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-52.rdu2.redhat.com [10.10.112.52]) by smtp.corp.redhat.com (Postfix) with ESMTP id D03DB600C9; Tue, 2 Jan 2018 19:07:08 +0000 (UTC) Subject: Re: RFA: Fix REG_ARGS_SIZE handling when pushing TLS addresses To: Andreas Schwab , gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org References: <871sltvm7r.fsf@linaro.org> <87h8upn5nm.fsf@linaro.org> <877etdhjl5.fsf@linaro.org> <87h8saio7a.fsf_-_@linaro.org> From: Jeff Law Message-ID: <765f10a7-aac8-2dc1-bd1e-7bedec6829d6@redhat.com> Date: Tue, 02 Jan 2018 19:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <87h8saio7a.fsf_-_@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00064.txt.bz2 On 12/28/2017 01:37 PM, Richard Sandiford wrote: > 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. OK. jeff