From patchwork Thu Jul 2 11:26:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 490589 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 EF1911402AD for ; Thu, 2 Jul 2015 21:26:41 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=C9mfyG+2; 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:date :message-id:from:to:cc:in-reply-to:subject:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=mit El5rFdaNaO5uESBGbqc4FnZmuCKRUAn08nZQvfoCm00uU1NScDLzOzZHwikMlb70 KpVfnREUtAxxX540DYhNzBbz6SCdjVAtuglMV9jwGQYc/lu3/dlGVqOik1U+MdD7 NJjO7Z4dLSCLtYFNO00L8Vm4p2yNmrbF/jYihU6E= 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:date :message-id:from:to:cc:in-reply-to:subject:mime-version :content-type:content-transfer-encoding; s=default; bh=vNxE8Ypin V0Iw6ZK4X/2Lmxwe70=; b=C9mfyG+2JCtWtqSgYLqWwHFncDg9tQ61sWurj2QrM 3is0NLmFdwuoHxE/g9p5+/C2cZDNy4YCvP5FBfR70EL7PHH0cdb1rtc8WZmNd0GH JlHRuNeq5jibXd4Ww9uVZBDkaYRTSv3v+P6q/q8QYGDioi47LFxMdnZ/p5ThZD6V Go= Received: (qmail 10933 invoked by alias); 2 Jul 2015 11:26:33 -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 10907 invoked by uid 89); 2 Jul 2015 11:26:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: bes.se.axis.com Received: from bes.se.axis.com (HELO bes.se.axis.com) (195.60.68.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 02 Jul 2015 11:26:30 +0000 Received: from localhost (localhost [127.0.0.1]) by bes.se.axis.com (Postfix) with ESMTP id 20C7B2E7EE; Thu, 2 Jul 2015 13:26:27 +0200 (CEST) Received: from bes.se.axis.com ([IPv6:::ffff:127.0.0.1]) by localhost (bes.se.axis.com [::ffff:127.0.0.1]) (amavisd-new, port 10024) with LMTP id avY9qlAL2+rS; Thu, 2 Jul 2015 13:26:26 +0200 (CEST) Received: from boulder.se.axis.com (boulder.se.axis.com [10.0.2.104]) by bes.se.axis.com (Postfix) with ESMTP id 069BA2E7B7; Thu, 2 Jul 2015 13:26:26 +0200 (CEST) Received: from boulder.se.axis.com (localhost [127.0.0.1]) by postfix.imss71 (Postfix) with ESMTP id E3D631105; Thu, 2 Jul 2015 13:26:25 +0200 (CEST) Received: from seth.se.axis.com (seth.se.axis.com [10.0.2.172]) by boulder.se.axis.com (Postfix) with ESMTP id D5FEF103D; Thu, 2 Jul 2015 13:26:25 +0200 (CEST) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by seth.se.axis.com (Postfix) with ESMTP id C8C723E049; Thu, 2 Jul 2015 13:26:25 +0200 (CEST) Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id t62BQPBc026644; Thu, 2 Jul 2015 13:26:25 +0200 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id t62BQPN6026640; Thu, 2 Jul 2015 13:26:25 +0200 Date: Thu, 2 Jul 2015 13:26:25 +0200 Message-Id: <201507021126.t62BQPN6026640@ignucius.se.axis.com> From: Hans-Peter Nilsson To: rdsandiford@googlemail.com CC: richard.sandiford@arm.com, gcc-patches@gcc.gnu.org In-reply-to: <87egkro9gc.fsf@googlemail.com> (message from Richard Sandiford on Wed, 1 Jul 2015 23:26:59 +0200) Subject: Fixed Regressions with "[committed] Use target-insns.def for prologue & epilogue insns" MIME-Version: 1.0 > From: Richard Sandiford > Date: Wed, 1 Jul 2015 23:26:59 +0200 > Hans-Peter Nilsson writes: > >> From: Richard Sandiford > >> Date: Tue, 30 Jun 2015 22:55:24 +0200 > > > >> Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu. > >> Also tested via config-list.mk. Committed as preapproved. > >> > >> Thanks, > >> Richard > >> > >> > >> gcc/ > >> * defaults.h (HAVE_epilogue, gen_epilogue): Delete. > >> * target-insns.def (epilogue, prologue, sibcall_prologue): New > >> targetm instruction patterns. > >> * alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_* > >> interface. > >> * calls.c (expand_call): Likewise. > >> * cfgrtl.c (cfg_layout_finalize): Likewise. > >> * df-scan.c (df_get_entry_block_def_set): Likewise. > >> (df_get_exit_block_use_set): Likewise. > >> * dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise. > >> * final.c (final_start_function): Likewise. > >> * function.c (thread_prologue_and_epilogue_insns): Likewise. > >> (reposition_prologue_and_epilogue_notes): Likewise. > >> * reorg.c (find_end_label): Likewise. > >> * toplev.c (process_options): Likewise. > > > > I think this one -being the most fitting patch in the range > > (225190:225210]- caused this regression for cris-elf: > > > > Running > > /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/cris-torture.exp > > ... > > FAIL: gcc.target/cris/torture/no-pro-epi-1.c -O3 -g (internal compiler error) > > FAIL: gcc.target/cris/torture/no-pro-epi-1.c -O3 -g (test for excess errors) > > > > This test checks that the -mno-prologue-epilogue option works, > > whose semantics is supposedly self-explanatory. > > Well, yes and no :-) Hm...I take that as an affirmation on the regression but perhaps a "no" to some of the my statements... > The crash is coming from the code that outputs > dwarf CFI information. The code that records this information is skipped > for targets without rtl prologues, with the comment: > > /* Targets which still implement the prologue in assembler text > cannot use the generic dwarf2 unwinding. */ > > That seems accurate. So what's -mno-prologue-epilogue supposed to do > wrt CFI entries? Should it output empty entries or none at all? A big "whatever" on that one. Debugging and omitting prologue and epilogue is not something I find reason to spend time on other than making sure there are no crashes. > The first-order reason for the failure is that the code used to be > conditional on #ifndef HAVE_prologue and didn't care what HAVE_prologue > itself evaluated to. So the condition on the pattern wasn't actually > tested. Not completely true: there was inconsistency between uses of #ifdef and if (HAVE_prologue). > Which I suppose leads to the question: does !HAVE_prologue when "prologue" > is defined mean "I know how to output rtl prologues, but the prologue > for this function is empty" or "I'll output the prologue as text rather > than rtl". I think it logically means the second. Agreed. > The condition says > whether the pattern can be used; if the pattern can be used but happens > to generate no code then it just outputs no instructions (which is pretty > common for prologues in leaf functions). > > The port seems to hedge its bets here. It has both: > > (define_expand "prologue" > [(const_int 0)] > "TARGET_PROLOGUE_EPILOGUE" > "cris_expand_prologue (); DONE;") > > and: > > void > cris_expand_prologue (void) > { > [...] > /* Don't do anything if no prologues or epilogues are wanted. */ > if (!TARGET_PROLOGUE_EPILOGUE) > return; Yeah, a visit to the archive supports me thinking this was an oversight, perhaps caused by the effects of the now fixed inconsistency. > which I guess means that the HAVE_prologue condition wasn't being > consistently tested. Now that it is: is -mno-prologue-epilogue > just supposed to generate empty prologues and epilogues, as implied > by the cris.c code? If so then removing the conditions on "prologue" > and "epilogue" should work. If not, then which of the targetm.have_prologue () > etc. conditions do you need to be true for -mno-prologue-epilogue? > > (You have the distinction of having the only port with conditional > prologue and epilogue patterns. :-)) Not any longer. Also removed a stale comment. This committed patch fixes the noted regressions, without causing further regressions, testing cris-elf in a simulator. gcc: * config/cris/cris.md ("epilogue"): Remove condition. ("prologue"): Ditto. brgds, H-P Index: config/cris/cris.md =================================================================== --- config/cris/cris.md (revision 225286) +++ config/cris/cris.md (working copy) @@ -3518,14 +3518,12 @@ (define_insn "*return_expanded" (define_expand "prologue" [(const_int 0)] - "TARGET_PROLOGUE_EPILOGUE" + "" "cris_expand_prologue (); DONE;") -;; Note that the (return) from the expander itself is always the last -;; insn in the epilogue. (define_expand "epilogue" [(const_int 0)] - "TARGET_PROLOGUE_EPILOGUE" + "" "cris_expand_epilogue (); DONE;") ;; Conditional branches.