From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15238 invoked by alias); 15 Jun 2010 14:09:08 -0000 Received: (qmail 15165 invoked by uid 22791); 15 Jun 2010 14:09:05 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 15 Jun 2010 14:08:59 +0000 Received: (qmail 15624 invoked from network); 15 Jun 2010 14:08:57 -0000 Received: from unknown (HELO ?192.168.0.104?) (mitchell@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Jun 2010 14:08:57 -0000 Message-ID: <4C178976.1060707@codesourcery.com> Date: Tue, 15 Jun 2010 14:52:00 -0000 From: Mark Mitchell User-Agent: Thunderbird 2.0.0.24 (Windows/20100228) MIME-Version: 1.0 To: Steven Bosscher CC: 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: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-06/txt/msg01508.txt.bz2 Steven Bosscher wrote: >>> 2010-06-14 Paul Brook >>> >>> gcc/ >>> * cse.c (cse_process_notes_1): Call simplify_rtx is a substitution >>> was made. > How is this obvious? > It makes me suspect that this change only papers over a bug elsewhere, > where a note replacement in an insn is not validated properly. I think we worry too much about that. Or, rather, we confuse *suboptimal* patches with *wrong* patches. A wrong patch is one that will make the compiler generate wrong code, or slow code, or cause an ICE, or do some other user-observable bad thing. A suboptimal patch is one for which there exists a superior implementation technique, possibly involving some much larger reworking of the sourcebase, that would produce a solution to more problems, a more elegant solution to the same problem, or in some other way be superior. Of course, I am all for superior patches, good infrastructure, clean-ups, and so forth. But, it seemed clear to me that (a) validate_change wasn't in the business of making canonical RTL, and (b) simplify_rtx was, and (c) this patch made the compiler better from a user-observable point of view, in that it eliminated crashes, and (d) there had been some previous discussion suggesting this approach, and (e) I couldn't see any user-observable harm it would do, and (f) it was a small local change. I think that should be good enough. Now, of course, if you have an idea about how to do it better, I think that's great! A more general, more elegant solution would be terrific. But, I don't want perfect to be the enemy of good. -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713