From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124682 invoked by alias); 23 Dec 2017 09:36:30 -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 124669 invoked by uid 89); 23 Dec 2017 09:36:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-9.4 required=5.0 tests=AWL,BAYES_40,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:Sat, sk:add_arg, 10927, sk:fixup_a X-HELO: mail-wr0-f176.google.com Received: from mail-wr0-f176.google.com (HELO mail-wr0-f176.google.com) (209.85.128.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 23 Dec 2017 09:36:27 +0000 Received: by mail-wr0-f176.google.com with SMTP id w68so17306378wrc.10 for ; Sat, 23 Dec 2017 01:36:26 -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=hlxYeNCK2Y6Vhxw/wKuYBWrCzZl+80e+Ci+ku8EVANI=; b=H1Xcw/5xIyluyni9v+s82ay/t8Vmp8ZbpcQFGmmydvcJj17vVFooi0qXFMqcj06gyt Htli2OHKhB+MWWJuWSKfP2Ok9u2E7Imnf7E6/dhCRlMEME7sydw2faS1zCaTqr5RjrXZ NAImjVAdAGWYtD3iQGQzbk3sxn29f49efEEYJmGiTjdRPY8crX5CCUAtzoHurIEYtT+C UfjwPn2B3diPg5OK7iXWaGNeq9hixAV8AqNyRM6Pf5i2z4fy1yeUWnpqHauoHsOYeLwN 8L0BtfGF1qbFdmmYmfIP/+90Ess0BAzCK7w/kuzdFYocCpAXuBdRkzGgeuV9SIYZBmIg kURQ== X-Gm-Message-State: AKGB3mL1GMSjPtb8lbEBKLp2bJuh3DowVgrQ1KoAgbJItfdwroNB2KrQ tiz/7XHTBNARW5HVpap3OeWT1KKNhaI= X-Google-Smtp-Source: ACJfBos0tbUuKhvkf4TfWC6IP/D3O9hFB5qlXLEPhhUH+jJThg47oZ14uVcMrCQqhAziZ3+n1ZvRnw== X-Received: by 10.223.141.133 with SMTP id o5mr18729666wrb.35.1514021784797; Sat, 23 Dec 2017 01:36:24 -0800 (PST) Received: from localhost ([95.147.106.85]) by smtp.gmail.com with ESMTPSA id i3sm7408088wre.33.2017.12.23.01.36.22 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 23 Dec 2017 01:36:23 -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: Re: [045/nnn] poly_int: REG_ARGS_SIZE References: <871sltvm7r.fsf@linaro.org> <87h8upn5nm.fsf@linaro.org> Date: Sat, 23 Dec 2017 09:36:00 -0000 In-Reply-To: (Andreas Schwab's message of "Fri, 22 Dec 2017 22:56:26 +0100") Message-ID: <877etdhjl5.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-12/txt/msg01502.txt.bz2 Andreas Schwab writes: > This breaks gcc.dg/tls/opt-3.c, gcc.dg/tls/pr47715-3.c and > gcc.dg/tls/struct-1.c on m68k: > > /daten/aranym/gcc/gcc-20171222/gcc/testsuite/gcc.dg/tls/opt-3.c:11:3: > internal compiler error: in add_args_size_note, at rtlanal.c:2379 > 0xae7aa9 add_args_size_note(rtx_insn*, poly_int<1u, long>) > ../../gcc/rtlanal.c:2379 > 0x7ea4ca fixup_args_size_notes(rtx_insn*, rtx_insn*, poly_int<1u, long>) > ../../gcc/expr.c:4105 > 0x7f6a02 emit_single_push_insn > ../../gcc/expr.c:4225 > 0x7fa412 emit_push_insn(rtx_def*, machine_mode, tree_node*, rtx_def*, > unsigned int, int, rtx_def*, poly_int<1u, long>, rtx_def*, rtx_def*, > int, rtx_def*, bool) > ../../gcc/expr.c:4561 > 0x6b8976 store_one_arg > ../../gcc/calls.c:5694 > 0x6c15b1 expand_call(tree_node*, rtx_def*, int) > ../../gcc/calls.c:4030 > 0x7f0485 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, > expand_modifier, rtx_def**, bool) > ../../gcc/expr.c:10927 > 0x6d6c97 expand_expr > ../../gcc/expr.h:276 > 0x6d6c97 expand_call_stmt > ../../gcc/cfgexpand.c:2690 > 0x6d6c97 expand_gimple_stmt_1 > ../../gcc/cfgexpand.c:3624 > 0x6d6c97 expand_gimple_stmt > ../../gcc/cfgexpand.c:3790 > 0x6d8058 expand_gimple_tailcall > ../../gcc/cfgexpand.c:3836 > 0x6d8058 expand_gimple_basic_block > ../../gcc/cfgexpand.c:5774 > 0x6dd62e execute > ../../gcc/cfgexpand.c:6403 Bah. Looks like I need to update my scripts to use --enable-tls, since I'd ended up with emultls for the m68k targets. I think the assert is catching a pre-existing bug here. 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 problem 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. This patch seems to fix it for me, but I'm not sure how best to test it. Thanks, Richard 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. */