From patchwork Wed Jun 27 10:06:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 167594 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 1A55DB7019 for ; Wed, 27 Jun 2012 20:06:39 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1341396400; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:From:To:Cc:Subject:References:Date: In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=I2jDOeQjqsCO05Kk9kSf 2U8z1zA=; b=Gj0ZzdfJ7KmU1Cg+YuXskoEne0EQxMEb7Z1hF1G7WRBY1nA+TmE0 AiD9i+TBToAl6gmX2jlMfQEzImTRh6/JIFQkz7lSZi4wRh+9/wI8zhfXBwUVh0F0 t20Dt+dwjbyJ5J3MDrHi6fBCj9te/xnNBYqxdxncWaBmkCDkW7uR8HY= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:From:To:Cc:Subject:References:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=At2H4mJe1clXgb0R1HEPczs21ZkG64mzuj7F3aRBK4evHBzIplHNYn84NFfNdJ vhT6CG78ox5AMzS0vNA+XPuM6zXj0S9n355abl1cHFUDAC4RSSNcwI60XrhmVI+x wwXZhHC7E+1SW37gHNVpyNP22F1L6aAWQau9OjnI3gQqs=; Received: (qmail 15226 invoked by alias); 27 Jun 2012 10:06:34 -0000 Received: (qmail 15214 invoked by uid 22791); 27 Jun 2012 10:06:33 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 27 Jun 2012 10:06:08 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5RA67cA020022 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 27 Jun 2012 06:06:07 -0400 Received: from freie (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5RA6544027423 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 27 Jun 2012 06:06:07 -0400 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie (8.14.5/8.14.5) with ESMTP id q5RA63tL025304; Wed, 27 Jun 2012 07:06:03 -0300 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id q5RA62i8013139; Wed, 27 Jun 2012 07:06:02 -0300 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id q5RA61ms013137; Wed, 27 Jun 2012 07:06:01 -0300 From: Alexandre Oliva To: gcc-patches@gcc.gnu.org Cc: rdsandiford@googlemail.com Subject: Re: RFA: dead_debug_* ICE References: <87lijd7zmj.fsf@talisman.home> Date: Wed, 27 Jun 2012 07:06:01 -0300 In-Reply-To: (Alexandre Oliva's message of "Mon, 25 Jun 2012 02:23:07 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 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 On Jun 25, 2012, Alexandre Oliva wrote: > On Jun 24, 2012, Richard Sandiford wrote: >> gcc.c-torture/compile/vector-2.c fails on mips64-elf with RTL checking >> enabled because dead_debug_insert_temp tries to read the REGNO of something >> that it has already replaced with a debug temporary. > This sounds like http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53740 > The patch looks reasonable to me, but I don't think I'm entitled to > approve it. So, it turned out that the observable problems were a red herring. The problem was that we were failing to emit debug temps for regs that were set *and* used, but that died before their last debug use. The “else” that I'd recently introduced was a mistake, for it stopped us from satisfying the needed debug binding with the correct expression if it had any nondebug uses. The need would get past (backwards) the setting point, reaching some earlier set that happened to be actually dead and getting us thoroughly confused. This was obviously not supposed to happen. Once I removed the incorrectly-added “else”s, the problem no longer occurred, and I'm pretty sure it won't. Now, while investigating the problem, I noticed a number of suspicious paradoxical SUBREGs that I'm pretty sure were supposed to refer to full double words rather than extending single words to double. Indeed, we had a problem in tracking single-reg sets for multi-reg debug uses: we'd use the value stored in the lowest-numbered as if it was the whole multireg value. This would be a sign extension of the low part given the right endianness, but it would become utter gibberish for the wrong one. There was code in place to avoid this incorrect use if we happened to be storing the part of the value in a SUBREG, but if we wrote to any entire REG, we'd happily do the wrong thing. I have plans on how to deal with multiregs properly, as noted in the newly-added comments, but I haven't decided it's worth tackling yet. Richard, is it easy for you to confirm that the patch fixes the mips64- problem too? If not, I'll build a cross and hope I can trigger the problem with it. (no chance of my building an rtx-checking native on my poor yeeloong ;-) This was regstrapped on x86_64- and i686-linux-gnu. I'm checking it in as obvious after catching some sleep. for gcc/ChangeLog from Alexandre Oliva PR debug/53740 PR debug/52983 PR debug/48866 * dce.c (word_dce_process_block): Check whether inserting debug temps are needed even for needed insns. (dce_process_block): Likewise. * df-problems.c (dead_debug_add): Add comment about multi-regs. (dead_debug_insert_temp): Likewise. Don't subreg when we're setting fewer regs than a multi-reg requires. Index: gcc/dce.c =================================================================== --- gcc/dce.c.orig 2012-06-27 02:29:32.290377543 -0300 +++ gcc/dce.c 2012-06-27 02:30:56.072721259 -0300 @@ -864,9 +864,12 @@ word_dce_process_block (basic_block bb, anything in local_live. */ if (marked_insn_p (insn)) df_word_lr_simulate_uses (insn, local_live); + /* Insert debug temps for dead REGs used in subsequent debug - insns. */ - else if (debug.used && !bitmap_empty_p (debug.used)) + insns. We may have to emit a debug temp even if the insn + was marked, in case the debug use was after the point of + death. */ + if (debug.used && !bitmap_empty_p (debug.used)) { df_ref *def_rec; @@ -963,9 +966,12 @@ dce_process_block (basic_block bb, bool anything in local_live. */ if (needed) df_simulate_uses (insn, local_live); + /* Insert debug temps for dead REGs used in subsequent debug - insns. */ - else if (debug.used && !bitmap_empty_p (debug.used)) + insns. We may have to emit a debug temp even if the insn + was marked, in case the debug use was after the point of + death. */ + if (debug.used && !bitmap_empty_p (debug.used)) for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++) dead_debug_insert_temp (&debug, DF_REF_REGNO (*def_rec), insn, DEBUG_TEMP_BEFORE_WITH_VALUE); Index: gcc/df-problems.c =================================================================== --- gcc/df-problems.c.orig 2012-06-27 02:30:45.000000000 -0300 +++ gcc/df-problems.c 2012-06-27 02:30:56.073721179 -0300 @@ -3179,6 +3179,9 @@ dead_debug_add (struct dead_debug *debug if (!debug->used) debug->used = BITMAP_ALLOC (NULL); + /* ??? If we dealt with split multi-registers below, we should set + all registers for the used mode in case of hardware + registers. */ bitmap_set_bit (debug->used, uregno); } @@ -3269,6 +3272,15 @@ dead_debug_insert_temp (struct dead_debu /* Hmm... Something's fishy, we should be setting REG here. */ if (REGNO (dest) != REGNO (reg)) breg = NULL; + /* If we're not overwriting all the hardware registers that + setting REG in its mode would, we won't know what to bind + the debug temp to. ??? We could bind the debug_expr to a + CONCAT or PARALLEL with the split multi-registers, and + replace them as we found the corresponding sets. */ + else if (REGNO (reg) < FIRST_PSEUDO_REGISTER + && (hard_regno_nregs[REGNO (reg)][GET_MODE (reg)] + != hard_regno_nregs[REGNO (reg)][GET_MODE (dest)])) + breg = NULL; /* Ok, it's the same (hardware) REG, but with a different mode, so SUBREG it. */ else