From patchwork Tue Dec 7 00:00:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sandra Loosemore X-Patchwork-Id: 48567 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D896D3858001 for ; Tue, 7 Dec 2021 00:01:24 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 7B9CE3858C60 for ; Tue, 7 Dec 2021 00:01:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B9CE3858C60 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: p9fpnGAAg01HuOyuqXxpZYRwkEevJTULGWcuytZ6aupRKfB/nuBoWbn/wChi5eX+rOUwvg3ZFh 6zycNc3gppHdilR09Zsh4TCn+6/wo+BXEKvJkH0iLrY2sxqIAK75ErHKPt0j9ZCwoXYTFD1juc 9UkpOPmJgdZulfFk8YCHyZZgplRs8GI5GEKHhhofpEGOK0BhozWxH7WNBYI4wwB2vIsxjS+R7e o5k7vn81qv+zpkgxXX/X2oUAWg4G9asVv2xC4HJZGlyrjPlJCkLZuomfvkb0qBI8VazkMoqyGf Qtwq72N0b6RE8hnjVph3zrCC X-IronPort-AV: E=Sophos;i="5.87,292,1631606400"; d="scan'208";a="69351425" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 06 Dec 2021 16:01:04 -0800 IronPort-SDR: tX3Do3aF6BsVy8U5mirmHcxuj12SI4Qvvp2YMS1faUNmeSSx9UlixPa64YanR/wSePqyjleAj/ XreGBLD/bj6om0KH2nvPjbS3B3roZKOF6Mh1lCa3JMJ8WOpA3glth9gdK3JoSC+RHpzVuH61EQ O7A/9CBpkVNTtDK0w5HbeeQNvBjG9ZpP/LCWcwmrwemAtX7/mL5kiMsvdj2f1NBZRJZBRXHQ47 i4N+QPCCsSbfVmj2RYBJGNNAb4RNGGvDdujC8Fs4VFymic7gnU/RDyu5xsDlTPMv5QJ/zej8ZO H6o= To: "gcc-patches@gcc.gnu.org" From: Sandra Loosemore Subject: [patch] lto: Don't run ipa-comdats pass during LTO Message-ID: Date: Mon, 6 Dec 2021 17:00:59 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 Content-Language: en-US X-ClientProxiedBy: svr-orw-mbx-12.mgc.mentorg.com (147.34.90.212) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" The attached patch fixes an ICE in lto1 at lto-partition.c:215 that was reported by a customer. Unfortunately I have no test case for this; the customer's application is a big C++ shared library with lots of dependencies and proprietary code under NDA. I did try reducing it with cvise but couldn't end up with anything small or robust enough to be suitable, and likewise my attempts to write a small testcase by hand were not successful in reproducing the bug. OTOH, I did track this down in the debugger and I think I have a pretty good understanding of what the problem is, namely... The symbol lto-partition was failing on was a vtable symbol that was in a comdat group but not marked as DECL_COMDAT and without the right visibility flags for a comdat symbol. The LTO data in the input .o files is correct and lto1 is creating the symbol correctly when it's read in, but the problem is that there are two optimization passes being run before lto-partition that are trying to do conflicting things with COMDATs. The first is the ipa-visibility pass. When this pass is run as part of lto1, since it has the complete program or shared library (as in this case) available to analyze, it tries to break up COMDATs and turn them into ordinary local symbols. But then the ipa-comdats pass, which is also run as part of regular IPA optimizations at compile time, tries to do the reverse and move local symbols only referenced from a COMDAT into that same COMDAT, but in the process it is failing to set all the flags needed by the LTO partitioner to correctly process the symbol. It's possible the failure to set the flags properly is a bug in ipa-comdats, but looking at the bigger picture, it makes no sense to be running this optimization pass at link time at all. It is a compile-time optimization intended to reduce code size by letting the linker keep only one copy of these symbols that may be redundantly defined in multiple objects. So the patch I've come up with disables the ipa-comdats pass in the same situations in which ipa-visibility localizes COMDAT symbols, to remove the conflict between the two passes. This fixes the customer problem (in a GCC 10 build for arm-linux-gnueabi), and regression tests OK on mainline x86_64-linux-gnu as well. OK for mainline, and to backport to older branches? It looked to me like none of this this code had been touched for years. -Sandra commit 559b1911c825c8df273d6cc7c403a3d0c79d0295 Author: Sandra Loosemore Date: Sun Dec 5 17:59:19 2021 -0800 lto: Don't run ipa-comdats pass during LTO When the ipa-visibility pass is invoked during LTO, it attempts to localize comdat symbols and destroy their comdat group. The ipa-comdats pass subsequently attempts to do the opposite and move symbols into comdats, but without marking them as DECL_COMDAT or restoring appropriate visibility flags. This confuses the lto-partition pass and leads to ICEs. Running the ipa-comdats pass at link time seems counterproductive in terms of optimization, so the simplest fix is not to do that. 2021-12-05 Sandra Loosemore gcc/ * ipa-comdats.c (pass_ipa_comdats::gate): Skip this pass if it would un-do the comdat localization already performed at LTO time in ipa-visibility.c. diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c index a9d2993..d178734 100644 --- a/gcc/ipa-comdats.c +++ b/gcc/ipa-comdats.c @@ -428,6 +428,13 @@ public: bool pass_ipa_comdats::gate (function *) { + if ((in_lto_p || flag_whole_program) && !flag_incremental_link) + /* This is the combination of flags cgraph_externally_visible_p in + ipa-visibility.c uses to enable localization of comdat symbols. + Do not attempt to undo that by trying to re-insert symbols into + comdats without their original visibility information, as it + causes assertion failures in lto-partition. */ + return false; return HAVE_COMDAT_GROUP; }