From patchwork Tue Jun 15 09:35:40 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 55621 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 ECA00B7D1B for ; Tue, 15 Jun 2010 19:36:01 +1000 (EST) Received: (qmail 9383 invoked by alias); 15 Jun 2010 09:35:59 -0000 Received: (qmail 9368 invoked by uid 22791); 15 Jun 2010 09:35:58 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM X-Spam-Check-By: sourceware.org Received: from mail-bw0-f47.google.com (HELO mail-bw0-f47.google.com) (209.85.214.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 15 Jun 2010 09:35:53 +0000 Received: by bwz14 with SMTP id 14so2978386bwz.20 for ; Tue, 15 Jun 2010 02:35:51 -0700 (PDT) Received: by 10.204.83.14 with SMTP id d14mr5259131bkl.50.1276594550484; Tue, 15 Jun 2010 02:35:50 -0700 (PDT) Received: from yakj.usersys.redhat.com (nat-pool-brq-t.redhat.com [209.132.186.34]) by mx.google.com with ESMTPS id v14sm23667770bkz.2.2010.06.15.02.35.42 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 15 Jun 2010 02:35:44 -0700 (PDT) Message-ID: <4C17496C.4070908@gnu.org> Date: Tue, 15 Jun 2010 11:35:40 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-3.fc13 Lightning/1.0b2pre Thunderbird/3.0.4 MIME-Version: 1.0 Newsgroups: gmane.comp.gcc.patches To: Steven Bosscher CC: Mark Mitchell , Paul Brook , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix CSE bogus RTL creation References: <201006150009.13542.paul@codesourcery.com> <4C16BE47.1090005@codesourcery.com> In-Reply-To: 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 06/15/2010 07:16 AM, Steven Bosscher wrote: > On Tue, Jun 15, 2010 at 1:41 AM, Mark Mitchell wrote: >> Paul Brook wrote: >> >>> 2010-06-14 Paul Brook >>> >>> gcc/ >>> * cse.c (cse_process_notes_1): Call simplify_rtx is a substitution >>> was made. >> >> I think this qualifies as near-obvious. OK. > > How is this obvious? > > 1. It means validate_change validates something that is not canonical RTL. But that is not the task of validate_change. It would be done with a patch like the attached (which is not meant to be applied right away, but would do the same as Paul's in a better way). > 2. You are canonicalizing something that is in a REG-note. How does > this non-canonical note content escape from the note into an INSN? Is > that not the point where this bug should be fixed? > > It makes me suspect that this change only papers over a bug elsewhere, > where a note replacement in an insn is not validated properly. It may still cause a missed optimization, so it's sensible to apply my patch or something like that after investigating (2). But I also would like to understand where is the note used in an insn, and why recog isn't trapping it there. Paolo Index: cse.c =================================================================== --- cse.c (revision 160609) +++ cse.c (working copy) @@ -6005,9 +6005,19 @@ cse_process_notes_1 (rtx x, rtx object, return x; case MEM: - validate_change (x, &XEXP (x, 0), - cse_process_notes (XEXP (x, 0), x, changed), 0); - return x; + { + int n = num_validated_changes (); + rtx new = cse_process_notes (XEXP (x, 0), x, changed); + validate_change (x, &XEXP (x, 0), new, 1); + if (verify_changes (n)) + { + if (!n) + confirm_change_group (); + } + else + cancel_changes (n); + return x; + } case EXPR_LIST: case INSN_LIST: @@ -6025,7 +6035,7 @@ cse_process_notes_1 (rtx x, rtx object, /* We don't substitute VOIDmode constants into these rtx, since they would impede folding. */ if (GET_MODE (new_rtx) != VOIDmode) - validate_change (object, &XEXP (x, 0), new_rtx, 0); + validate_change (object, &XEXP (x, 0), new_rtx, object != NULL_RTX); return x; } @@ -6057,8 +6067,11 @@ cse_process_notes_1 (rtx x, rtx object, for (i = 0; i < GET_RTX_LENGTH (code); i++) if (fmt[i] == 'e') validate_change (object, &XEXP (x, i), - cse_process_notes (XEXP (x, i), object, changed), 0); + cse_process_notes (XEXP (x, i), object, changed), 1); + canonicalize_change_group (object, x); + if (object == NULL_RTX) + apply_change_group (); return x; }