From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by sourceware.org (Postfix) with ESMTPS id 4F0D53858D20 for ; Tue, 8 Nov 2022 14:22:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F0D53858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x62a.google.com with SMTP id 13so39089580ejn.3 for ; Tue, 08 Nov 2022 06:22:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=iYPjxtuSt0lzTJjpzpgmrKCfTVGIux6hgkC5Z2aaWdo=; b=XnYBmbYTxuKxgIduJObXE+V01cfTphVscCQLy+SO+p64VXODt41bGcexJNcw7LG2sG hfy3GqkESTcsUXDDXF7OIB+Qg0FYYxvVKANxSOen32XS9b+iFnmyHk78Sw/kA9v6B9+f STX5jQRazByUSNiRNA4KehrZyC1/rzmVGQjDWBQtkMFN3tLeXxG2ygZ28Wf5AwOeiSsV uKyrDrWY1g69dmeoxNexbKvTUHHhVbpLmzvvQ8QySmRk1N+efCMdMdSeukeVefxiNto0 5LJ/qowP2reWwo8hKnIJp0nFJlQF9aHu2caeI+wizFewLJlmaTJsBnSb59p6J8BWnmjq QjOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=iYPjxtuSt0lzTJjpzpgmrKCfTVGIux6hgkC5Z2aaWdo=; b=EukOdpw9fjwCCGdtPONCQCSz22J9wQ7IPilnQLAYLkcB5CoFCffjzIUEF6ESmka8Pm LEB1PHbGqXiCJTlgAlMVNZS7RWQA2vX6ha+MiHJuy+q+SMc0Q953q8z/i0AXRqY7ri9u HlJwxAAivy29WnpcubduxP/Ik4MoFv9+l9drtozlw4fq0je+mpHY2cafFFOvulIgQF0N O8qcuboU+FdIfMlr5bVE75a3edfFAuw5jlQXd8dEI1ritlDK7+kk8AJS7nUkObxis9kP /or4dY+V4ponEJrEz7GV76vENDyoFxV1esubS87iqKjbOPGKTlHm+oNkBsUqOuNu4iSX Priw== X-Gm-Message-State: ANoB5pndVzBaff5uELZHGRfYUO29fpoZT4JUvNFB3mABIKCMYGi7KNds 62QKlzCrn1NsMYF3dHZqi2VYNg+ELtn0ZCFPp+Q= X-Google-Smtp-Source: AA0mqf4ren0j2+9MHkXOVPcuAbeB9+GDyy8SGcrvJFCc+6NnoYL9noaj2j/eCY10cUd2WyZ32pwh3aqYIitx+B1Ddrs= X-Received: by 2002:a17:907:778a:b0:7ae:743c:61c1 with SMTP id ky10-20020a170907778a00b007ae743c61c1mr6756129ejc.511.1667917345012; Tue, 08 Nov 2022 06:22:25 -0800 (PST) MIME-Version: 1.0 References: <06ea9c1bd7e9b1493a1e740d8b6cf6f72be3db3e.1666220603.git.lhyatt@gmail.com> <20221103200704.GA86831@ldh-imac.local> In-Reply-To: <20221103200704.GA86831@ldh-imac.local> From: Richard Biener Date: Tue, 8 Nov 2022 15:22:13 +0100 Message-ID: Subject: Re: [PATCH] diagnostics: Allow FEs to keep customizations for middle end [PR101551, PR106274] To: Lewis Hyatt Cc: Jan Hubicka , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Nov 3, 2022 at 9:07 PM Lewis Hyatt wrote: > > On Fri, Oct 28, 2022 at 10:28:21AM +0200, Richard Biener wrote: > > Yes, the idea was also to free up memory but then that part never > > really materialized - the idea was to always run free-lang-data, not > > just when later outputting LTO bytecode. The reason is probably > > mainly the diagnostic regressions you observe. > > > > Maybe a better strathegy than your patch would be to work towards > > that goal but reduce the number of "freeings", instead adjusting the > > LTO streamer to properly ignore frontend specific bits where clearing > > conflicts with the intent to preserve accurate diagnostics throughout > > the compilation. > > > > If you see bits that when not freed would fix some of the observed > > issues we can see to replicate the freeing in the LTO output machinery. > > > > Richard. > > Thanks again for the suggestions. I took a look and it seems pretty doable to > just stop resetting all the diagnostics hooks in free-lang-data. Once that's > done, the only problematic part that I have been able to identify is here in > ipa-free-lang-data.c around line 674: > > ==== > /* We need to keep field decls associated with their trees. Otherwise tree > merging may merge some fields and keep others disjoint which in turn will > not do well with TREE_CHAIN pointers linking them. > > Also do not drop containing types for virtual methods and tables because > these are needed by devirtualization. > C++ destructors are special because C++ frontends sometimes produces > virtual destructor as an alias of non-virtual destructor. In > devirutalization code we always walk through aliases and we need > context to be preserved too. See PR89335 */ > if (TREE_CODE (decl) != FIELD_DECL > && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL) > || (!DECL_VIRTUAL_P (decl) > && (TREE_CODE (decl) != FUNCTION_DECL > || !DECL_CXX_DESTRUCTOR_P (decl))))) > DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl)); > ==== > > The C++ implementations of the decl_printable_name langhook and the diagnostic > starter callback do not work as-is when the DECL_CONTEXT for class member > functions disappears. So I did have a patch that changes the C++ > implementations to work in this case, but attached here is a new one along the > lines of what you suggested, rather changing the above part of free-lang-data > so it doesn't activate as often. The patch is pretty complete (other than > missing a commit message) and bootstrap + regtest all languages looks good > with no regressions. I tried the same with BUILD_CONFIG=bootstrap-lto as well, > and that also looked good when it eventually finished. I added testcases for > several frontends to verify that the diagnostics still work with -flto. I am > not sure what are the implications for LTO itself, of changing this part of > the pass, so I would have to ask you to weigh in on that aspect please. Thanks! First of all sorry for the delay and thanks for trying. The effect on LTO is an increase in the amount of streamed IL since we follow the DECL_CONTEXT edge when streaming the tree graph. So my solution for this would be to reflect the case you remove in free-lang-data in both lto-streamer-out.cc:DFS::DFS_write_tree_body where we do if (TREE_CODE (expr) != TRANSLATION_UNIT_DECL && ! DECL_CONTEXT (expr)) DFS_follow_tree_edge ((*all_translation_units)[0]); else DFS_follow_tree_edge (DECL_CONTEXT (expr)); and in tree-streamer-out.cc:write_ts_decl_minimal_tree_pointers which does if (TREE_CODE (expr) != TRANSLATION_UNIT_DECL && ! DECL_CONTEXT (expr)) stream_write_tree_ref (ob, (*all_translation_units)[0]); else stream_write_tree_ref (ob, DECL_CONTEXT (expr)); that possibly boils down to "just" doing tree ctx = DECL_CONTEXT (..); if (TREE_CODE (..) == VAR_DECL || TREE_CODE (..) == FUNCTION_DECL) ctx = fld_decl_context (ctx); and using 'ctx' for DECL_CONTEXT in those two places (and exporting the fld_decl_context function). As said the idea for this is that we want to avoid streaming type trees when not necessary. When doing an LTO bootstrap with your patch you should see (slightly) larger object files. Richard. > > -Lewis