From patchwork Wed Dec 23 18:25:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 560677 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8B19D140B9B for ; Thu, 24 Dec 2015 05:25:57 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=yH3xTwNt; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=w7cfvZAEfHahFVQfo dF+Ab189Ggz59PCLqDBUroqCmfgZfV98+6ZyGva9EGyzsobscLaEvfCE2KDjk9vQ 5Zf5zVJw4/rY2m2W2HQGPfGdfwcTuECoUS/v/h3QDj0H05MK0294/uyZUWZ6eMPC 47gIQDX8uYxiiyiNGq4LQqQIgo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=+7fs/qZItgDGrPdH64bgkbc Brt8=; b=yH3xTwNtQ8h4JPj2mDyTrLsP1tA2Wb5h4qUIpbbeQKxP02Pp5Gs7x2R cKDC+8OpPfGYf95ayzF5Ud6PMwykAWC1n4kf3YrIshwjW45vpqgz7/ssARtQHWw3 3AyH3ugqi8aL/PX4TyVmcbstMA/f91NvWEXRpbLXRR5+eXyEI5ZE= Received: (qmail 32683 invoked by alias); 23 Dec 2015 18:25:50 -0000 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 Received: (qmail 32669 invoked by uid 89); 23 Dec 2015 18:25:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=BAYES_50, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 spammy=D*acm.org, nathan@acm.org, nathanacmorg, sidwell X-HELO: mail-qg0-f48.google.com Received: from mail-qg0-f48.google.com (HELO mail-qg0-f48.google.com) (209.85.192.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 23 Dec 2015 18:25:48 +0000 Received: by mail-qg0-f48.google.com with SMTP id 6so4834974qgy.1 for ; Wed, 23 Dec 2015 10:25:47 -0800 (PST) X-Received: by 10.140.130.22 with SMTP id 22mr42657473qhc.99.1450895145910; Wed, 23 Dec 2015 10:25:45 -0800 (PST) Received: from ?IPv6:2601:181:c000:c497:a2a8:cdff:fe3e:b48? ([2601:181:c000:c497:a2a8:cdff:fe3e:b48]) by smtp.googlemail.com with ESMTPSA id o14sm418824qkl.30.2015.12.23.10.25.45 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 23 Dec 2015 10:25:45 -0800 (PST) Subject: Re: varpool/constpool bug To: Jeff Law , GCC Patches References: <56758A2A.4090206@acm.org> <567856CE.90908@redhat.com> <567AB446.107@acm.org> Cc: seurer@linux.vnet.ibm.com From: Nathan Sidwell Message-ID: <567AE728.7020003@acm.org> Date: Wed, 23 Dec 2015 13:25:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <567AB446.107@acm.org> On 12/23/15 09:48, Nathan Sidwell wrote: > On 12/21/15 14:45, Jeff Law wrote: > >> With some kind of comment in decl_in_symtab_p indicating why we need to check >> and filter on !DECL_IN_CONSTANT_POOL this is OK. > > Done, thanks. (I half guessed HPPA might be such a port :) sigh, there are other routes by which constpool entries get into the varpool, and it appears other checks using symtab_in_varpool_p to figure out whether they need to do something. Leading to powerpc having a problem. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69033 powerpc inserts an array initializer constant pool entry into the varpool via tree_output_constant_def (varasm.c), that has an explicit call to: varpool_node::finalize_decl (decl); so it seems intentional that constpool objects can end up also in the varpool. I notice that via this route varpool thinks the constant's value is available, which is different to the behaviour when insertionwas via alias checking. So, I think that either 1) alias checking shouldn't create varpool entries (i.e. use varpool::get rather than varpool::get_create). It wouldn't need to use the symtab_in_varpool_p predicate in that case. 2) Or varpool::get_create should take note of DECL_IN_CONSTANT_POOL and declare the value available 3) or build_constant_desc (varasm.c) should explicitly put the constant in the varpool, to match tree_output_constant_def? I have reverted the cgraph.h change and am experimenting with option #1 nathan 2015-12-23 Nathan Sidwell gcc/ * cgraph.h (decl_in_symtab_p): Revert check DECL_IN_CONSTANT_POOL. gcc/testsuite/ * gcc.dg/alias-15.c: Revert. Index: cgraph.h =================================================================== --- cgraph.h (revision 231929) +++ cgraph.h (working copy) @@ -2294,19 +2294,13 @@ symtab_node::real_symbol_p (void) } /* Return true if DECL should have entry in symbol table if used. - Those are functions and static & external non-constpool variables. - We do not expect constant pool variables in the varpool, as they're - not related to other variables, and simply lazily inserting them - using the regular interface results in varpool thinking they are - externally provided -- which results in erroneous assembly emission - as an undefined decl. */ + Those are functions and static & external veriables*/ static inline bool decl_in_symtab_p (const_tree decl) { return (TREE_CODE (decl) == FUNCTION_DECL || (TREE_CODE (decl) == VAR_DECL - && !DECL_IN_CONSTANT_POOL (decl) && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))); } Index: testsuite/gcc.dg/alias-15.c =================================================================== --- testsuite/gcc.dg/alias-15.c (revision 231929) +++ testsuite/gcc.dg/alias-15.c (working copy) @@ -1,15 +0,0 @@ -/* { dg-do compile } */ -/* { dg-additional-options "-O2 -fdump-ipa-cgraph" } */ - -/* RTL-level CSE shouldn't introduce LCO (for the string) into varpool */ -char *p; - -void foo () -{ - p = "abc\n"; - - while (*p != '\n') - p++; -} - -/* { dg-final { scan-ipa-dump-not "LC0" "cgraph" } } */