From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16834 invoked by alias); 5 Nov 2007 19:20:02 -0000 Received: (qmail 16763 invoked by uid 22791); 5 Nov 2007 19:20:00 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 05 Nov 2007 19:19:58 +0000 Received: (qmail 25369 invoked from network); 5 Nov 2007 19:19:56 -0000 Received: from unknown (HELO ?192.168.0.3?) (mitchell@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Nov 2007 19:19:56 -0000 Message-ID: <472F6CD4.3010408@codesourcery.com> Date: Mon, 05 Nov 2007 19:20:00 -0000 From: Mark Mitchell User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Alexandre Oliva CC: gcc-patches@gcc.gnu.org, dj@redhat.com Subject: Re: stabilize .gcc_except_table with or without -g References: 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: 2007-11/txt/msg00213.txt.bz2 Alexandre Oliva wrote: > I haven't completed bootstrap-testing of this patch yet, but it has > shaved off more than half of the remaining compare-debug differences > between libjava compiled with -fvar-tracking-assignments enabled or > disabled on x86_64-linux-gnu. > > Ok to install if it passes bootstrap? Should new the compare function > be moved into libiberty proper? Does the assertion check seem safe in > the context of this use, that when pointers differ then the strings > they point to must differ as well? It's not necessary, but I thought > it would be nice to make sure we weren't goofing with identical symbol > names at different locations before. I could be easily convinced to > take it out, though. Thanks for working on this issue. +/* Compare C strings used as keys in a splay tree, optimizing the case + in which the pointers are identical. We need to use string rather + than pointer comparison in order to make Java exception action + records stable in the presence of different debug info options. */ I think a lot of this comment is unhelpful. I would say: /* Comparison function for a splay tree in which the keys are strings. K1 and K2 have the dynamic type "const char *". Returns <0, 0, or >0 to indicate whether K1 is less than, equal to, or greater than K2, respectively. */ The optimization isn't important to a reader trying to understand the function is for. The Java comment is confusing, in that it might lead you to think that without Java we don't need to do this -- but, quite probably, we do. In the body of the function, you could write: /* We use strcmp, rather than just comparing pointers, so that the sort order will not depend on the host system. */ If you leave the assertion in, you definitely should comment: /* The strings are always those from IDENTIFIER_NODEs, and, therefore, we should never have two copies of the same string. */ The patch is OK with those changes. If DJ prefers to put this function in libiberty, that's fine; if you check it into GCC first, consider a patch to adjust GCC to call into libiberty pre-approved. Thanks, -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713