From patchwork Wed Dec 30 20:21:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 561806 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 ABF2C140C12 for ; Thu, 31 Dec 2015 07:21:44 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=IVXzPI4I; dkim-atps=neutral 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:subject:date:message-id:references:in-reply-to:content-type :mime-version; q=dns; s=default; b=FhvmL6XG9+wUlkTOFJMH1RInGfKgL xVHR5RO5xxrRGXe5i3bQ+b4eIZ/2pHi6aZ1WY4ziEXNSVMT89vrVcjOK0IDpyjyS xyJ3OHHE9RoRNRwefdKkQG/aJaDuCkAIxKc8l/DwYgAKqXUIDbd8QWIe0hPtklH4 mpkYMWi5E3nKew= 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:subject:date:message-id:references:in-reply-to:content-type :mime-version; s=default; bh=8cAJQLhCdljjLtpGCZe3CxjKtDU=; b=IVX zPI4IUAU9faSSFXs130QaaHQMw3KRmepAxj6NWkTxB6erHO4MbrtSnH41wlrx+Zn nuS0JhZDXc7xehAP+UbAQZfUfulshiqmgM9TxR/B+jCIjA199oXnXp/8o6UmRL46 u73lAdlUwYFfVVU5CjKs0ktCGUJptVNoa1z2jGpA= Received: (qmail 7795 invoked by alias); 30 Dec 2015 20:21:37 -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 7786 invoked by uid 89); 30 Dec 2015 20:21:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_20, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=host_wide_int, HOST_WIDE_INT, regno, cached X-HELO: DUB004-OMC1S31.hotmail.com Received: from dub004-omc1s31.hotmail.com (HELO DUB004-OMC1S31.hotmail.com) (157.55.0.230) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA256 encrypted) ESMTPS; Wed, 30 Dec 2015 20:21:36 +0000 Received: from emea01-am1-obe.outbound.protection.outlook.com ([157.55.0.237]) by DUB004-OMC1S31.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.23008); Wed, 30 Dec 2015 12:21:32 -0800 Received: from HE1PR07MB0908.eurprd07.prod.outlook.com (10.162.26.15) by HE1PR07MB1193.eurprd07.prod.outlook.com (10.163.178.23) with Microsoft SMTP Server (TLS) id 15.1.361.13; Wed, 30 Dec 2015 20:21:32 +0000 Received: from HE1PR07MB0905.eurprd07.prod.outlook.com (10.162.26.12) by HE1PR07MB0908.eurprd07.prod.outlook.com (10.162.26.15) with Microsoft SMTP Server (TLS) id 15.1.361.13; Wed, 30 Dec 2015 20:21:31 +0000 Received: from HE1PR07MB0905.eurprd07.prod.outlook.com ([10.162.26.12]) by HE1PR07MB0905.eurprd07.prod.outlook.com ([10.162.26.12]) with mapi id 15.01.0361.006; Wed, 30 Dec 2015 20:21:31 +0000 From: Bernd Edlinger To: "gcc-patches@gcc.gnu.org" , Matthew Fortune , "rdsandiford@googlemail.com" Subject: Re: [PATCH] Fix pr69012 ICE on building libgfortran for mips Date: Wed, 30 Dec 2015 20:21:31 +0000 Message-ID: References: <8760zgovn0.fsf@googlemail.com> In-Reply-To: <8760zgovn0.fsf@googlemail.com> authentication-results: gcc.gnu.org; dkim=none (message not signed) header.d=none; gcc.gnu.org; dmarc=none action=none header.from=hotmail.de; x-ms-exchange-messagesentrepresentingtype: 1 x-microsoft-exchange-diagnostics: 1; HE1PR07MB0908; 23:C9Hh+YsEnyjWC+SVF6aiRM5qNTBLOIBe388dXTx+oxWhxogG+hLPC7nXNg1Pnf8hvcKI5kT8e40T1bFf/D2SulE1dIo3zoV0Ho9HKYRsFKa0l5emI6rHrqWOCpEuju1mj+vSN44IhmELI4tky4/N63Ln8evsCzUWIcV2jGj1JJw86ilNzmLXBAmuju+0dpX58X5ULqvmSK9q5dsLo1NWrQ==; 5:a3VGQUHHuCgM1BqVTHSjUu3mIzJlTJyxth+KiWuJ8GvWXVicBkoiSqnuqNHUMrIBlzAgYL0o5s7NSK8lrHD2GYShWY8PAJgYU9mKsS9qPRgsEgTEmyimk4q5MNfUd0NJQ38PUaaDghtanMGeYqOgyg==; 24:ef3EMjUfgLk0nBgHxWYKLfeZodQCTSfbqVY8n9EhekMp7TTz/MoeKZRggkVZUn4FFCQZiJA6GyVq3RpMd34anQFJQyM3JIodybVIz1Xon70= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR07MB0908; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(432015012)(82015046); SRVR:HE1PR07MB0908; BCL:0; PCL:0; RULEID:; SRVR:HE1PR07MB0908; x-forefront-prvs: 08062C429B x-forefront-antispam-report: SFV:NSPM; SFS:(7070004)(98900002); DIR:OUT; SFP:1901; SCL:1; SRVR:HE1PR07MB0908; H:HE1PR07MB0905.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Dec 2015 20:21:31.0886 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR07MB0908 X-Microsoft-Exchange-Diagnostics: 1; HE1PR07MB1193; 2:Px3ez9OwYmC489xwahD9x0JKyRIpF0kb46ATbf9p6PGkrrsS/cgB6PBb9n2o+BoB2iPaw4r5UgL/RWrCY+XXByRX7jsiiL/e1biwE7fRsQvtpIwSNnRcaI5U/cq9+18cC/ex0E99lOJLY3cJMJ38Sg==; 23:5HTAbhBPx0PfqVXNNDcqqJMGrqfGuD6k6Ktr0iXwEAegjFyiL/tkolmQg9bjgHeX354nXAzZX5I4vNu1YqJ9g+1a5zLe3aRQzrx/SRkZDWPCDeu9DRWRqSUVuCimaNES0uPbHs03l/NZWQxQQLz2A5f9urL3lDnMj4V7usMhGAM= X-OriginatorOrg: sct-15-1-318-9-msonline-outlook-efc2f.templateTenant Hi, On 30.12.2015 15:31, Richard Sandiford wrote: > I think the problem is deeper than that though. The instructions that > are triggering the ICE are only generated by the prologue, so this > means that we're trying to lay out the frame again after the prologue > has been generated, whereas it really needs to be fixed by then. (And > even if recalculating it is a no-op, the operation is still too > expensive to be repeated lightly.) Query functions like > rtx_addr_can_trap_p(_1) shouldn't really be changing or recalculating > the frame layout or other global state. I think we need to find a > different way of getting the information. Maybe reload/LRA should use > its own structures to calculate the range of "safe" stack and hfp > offsets, then store them in crtl for rtx_addr_can_trap_p to use. AIUI, > before reload, the only non-trapping uses of sp should be for outgoing > arguments, so we can use a test based on the cumulative outgoing > arguments size. I don't think the hfp should be used at all before > reload, so we could conservatively return -1 for that case. Thanks, > Richard Yes, I agree, it _should_ be a no-op, but given the complexity of mips_compute_frame_info it is probably better to use cached values after reload completed. Before reload_completed, rtx_addr_can_trap_p is just trying to do an initial guess on the stack layout, it may well be possible that some important information is still missing here. We will never call the target callback from here, before reload_completed. It is however not completely impossible that rtx_addr_can_trap_p is called while lra/reload is already renaming the registers, see PR66614. Of course the required information is already cached in cfun->machine->frame, but rtx_addr_can_trap_p can't use the information without a target callback. And I just want to avoid to invent a new callback for that. I looked at how other targets handle this situation and found that most of them either have a more simple frame layout function or they keep the previously cached values if they are ever called after reload_completed. I found, previously mips_compute_frame_info was always called with reload_completed=false, either from mips_frame_pointer_required or from mips_initial_elimination_offset. Reload seems to call the initial_elimination_offset repeatedly and seems to _expect_ the values to change from time to time. However it does not seem to expect the result from mips_frame_pointer_required to change after reload/LRA has started. But as it seems, that may be possible for TARGET_MIPS16 if the frame size happens to be very close to 0x7FFF. Well that could be another story. Anyways when mips_compute_frame_info is called with reload_completed=true, we can as well use the cached values, as they were immediately before reload_completed. So what do you think of my new version of the patch? Thanks Bernd. 2015-12-30 Bernd Edlinger PR target/69012 * config/mips/mips.c (mips_compute_frame_info): Don't change the frame info after reload completed. Index: gcc/config/mips/mips.c =================================================================== --- gcc/config/mips/mips.c (revision 231954) +++ gcc/config/mips/mips.c (working copy) @@ -10321,6 +10321,10 @@ mips_compute_frame_info (void) HOST_WIDE_INT offset, size; unsigned int regno, i; + /* Don't change the frame info after reload completed. */ + if (reload_completed) + return; + /* Set this function's interrupt properties. */ if (mips_interrupt_type_p (TREE_TYPE (current_function_decl))) {