From patchwork Wed Sep 15 22:54:56 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 64925 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]) by ozlabs.org (Postfix) with SMTP id ED117B6F06 for ; Thu, 16 Sep 2010 08:54:31 +1000 (EST) Received: (qmail 6456 invoked by alias); 15 Sep 2010 22:54:30 -0000 Received: (qmail 6441 invoked by uid 22791); 15 Sep 2010 22:54:28 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_20 X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (212.99.106.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 15 Sep 2010 22:54:23 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 244C8CB01E0 for ; Thu, 16 Sep 2010 00:54:21 +0200 (CEST) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1vCjWKCqUDTc for ; Thu, 16 Sep 2010 00:54:21 +0200 (CEST) Received: from [192.168.1.2] (bon31-9-83-155-120-49.fbx.proxad.net [83.155.120.49]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id E2633CB01D4 for ; Thu, 16 Sep 2010 00:54:20 +0200 (CEST) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: Fix PR rtl-optimization/45593 Date: Thu, 16 Sep 2010 00:54:56 +0200 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Disposition: inline Message-Id: <201009160054.56596.ebotcazou@adacore.com> 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 This is a segfault compiling the kernel at -Os for the SPARC, a regression present on the mainline and 4.5 branch. The crash occurs during delay slot scheduling, aka reorg. We start with an insn that is put into a delay slot by fill_simple_delay_slots during the first reorg pass; it is only copied and not deleted. Then it is put into a second delay slot by fill_eager_delay_slots, re-copied and still not deleted. Then fill_eager_delay_slots, upon attempting to put it into a third delay slot, notices that the insn is now redundant with the copy of itself(!) put into the second delay slot and finally deletes it from its original place. Then relax_delay_slots kicks in and remarks that the cond jump owning the second delay slot jumps to the next instruction; therefore it re-emits the insn that was in the second delay slot as a stand-alone insn and deletes the jump. In the end, the insn has been copied into a delay slot and hoisted in the stream; in particular, its basic block changed. These transformations are interleaved with queries to mark_target_live_regs which needs to know for each insn the basic block it can use to start computing liveness info from. The results are cached in a hash table indexed by the INSN_UID of the insns. Although insns are copied as a whole when they are put into delay slots, INSN_UID included, this works because m_t_l_r doesn't look within SEQUENCEs to compute the basic block; only the enclosing insn is looked at and these insns don't move. Things go awry when relax_delay_slots re-emits insns that were in delay slots as stand-alone insns, because they are re-emitted as-is and thus we end up with several top-level insns with the same INSN_UID in the stream, which of course mightily confuses the caching done for mark_target_live_regs. That's what happens here and causes the crash during the second reorg pass. A radical fix could be to hash the insns themselves instead of their INSN_UID but that's probably overkill. So the patch just makes sure the INSN_UID of the insns is changed where they are re-emitted in relax_delay_slots. Bootstrapped/regtesed on sparc-sun-solaris2.8 and sparc64-sun-solaris2.9, applied on the mainline and 4.5 branch. 2010-09-15 Eric Botcazou PR rtl-optimization/45593 * reorg.c (relax_delay_slots): Use emit_copy_of_insn_after to re-emit insns that were in delay slots as stand-alone insns. 2010-09-15 Eric Botcazou * gcc.c-torture/compile/20100915-1.c: New test. Index: reorg.c =================================================================== --- reorg.c (revision 164211) +++ reorg.c (working copy) @@ -3459,9 +3459,13 @@ relax_delay_slots (rtx first) We do this by deleting the INSN containing the SEQUENCE, then re-emitting the insns separately, and then deleting the RETURN. This allows the count of the jump target to be properly - decremented. */ + decremented. - /* Clear the from target bit, since these insns are no longer + Note that we need to change the INSN_UID of the re-emitted insns + since it is used to hash the insns for mark_target_live_regs and + the re-emitted insns will no longer be wrapped up in a SEQUENCE. + + Clear the from target bit, since these insns are no longer in delay slots. */ for (i = 0; i < XVECLEN (pat, 0); i++) INSN_FROM_TARGET_P (XVECEXP (pat, 0, i)) = 0; @@ -3469,13 +3473,10 @@ relax_delay_slots (rtx first) trial = PREV_INSN (insn); delete_related_insns (insn); gcc_assert (GET_CODE (pat) == SEQUENCE); - after = trial; - for (i = 0; i < XVECLEN (pat, 0); i++) - { - rtx this_insn = XVECEXP (pat, 0, i); - add_insn_after (this_insn, after, NULL); - after = this_insn; - } + add_insn_after (delay_insn, trial, NULL); + after = delay_insn; + for (i = 1; i < XVECLEN (pat, 0); i++) + after = emit_copy_of_insn_after (XVECEXP (pat, 0, i), after); delete_scheduled_jump (delay_insn); continue; } @@ -3580,9 +3581,13 @@ relax_delay_slots (rtx first) We do this by deleting the INSN containing the SEQUENCE, then re-emitting the insns separately, and then deleting the jump. This allows the count of the jump target to be properly - decremented. */ + decremented. - /* Clear the from target bit, since these insns are no longer + Note that we need to change the INSN_UID of the re-emitted insns + since it is used to hash the insns for mark_target_live_regs and + the re-emitted insns will no longer be wrapped up in a SEQUENCE. + + Clear the from target bit, since these insns are no longer in delay slots. */ for (i = 0; i < XVECLEN (pat, 0); i++) INSN_FROM_TARGET_P (XVECEXP (pat, 0, i)) = 0; @@ -3590,13 +3595,10 @@ relax_delay_slots (rtx first) trial = PREV_INSN (insn); delete_related_insns (insn); gcc_assert (GET_CODE (pat) == SEQUENCE); - after = trial; - for (i = 0; i < XVECLEN (pat, 0); i++) - { - rtx this_insn = XVECEXP (pat, 0, i); - add_insn_after (this_insn, after, NULL); - after = this_insn; - } + add_insn_after (delay_insn, trial, NULL); + after = delay_insn; + for (i = 1; i < XVECLEN (pat, 0); i++) + after = emit_copy_of_insn_after (XVECEXP (pat, 0, i), after); delete_scheduled_jump (delay_insn); continue; }