From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 4FE713858421; Mon, 26 Jun 2023 06:39:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4FE713858421 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1687761579; bh=blekAjPNlELvkaA9lgP2PFtGr1pW7UytXhCBwf0DvXM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=J9IRe3WZiUeKL/vUumRGHmtrzLocYcuF4UJzOpJ/M3xYGe31QhQaNYw7JfXbRRbGY dgQKRLzscLWpxvxdgDAXgSn+krZRNNAKDWT9pGJHlrlfvG+sSKQ2Egiojd/2f1zwTS Uvz+xDsr8tRQmPGARXY0APeOfUWJxpvHkjGDd0qk= From: "rguenther at suse dot de" To: gcc-bugs@gcc.gnu.org Subject: [Bug ipa/110334] [13/14 Regresssion] unused functions not eliminated before LTO streaming Date: Mon, 26 Jun 2023 06:39:36 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: ipa X-Bugzilla-Version: 13.1.1 X-Bugzilla-Keywords: lto, missed-optimization X-Bugzilla-Severity: normal X-Bugzilla-Who: rguenther at suse dot de X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: 13.2 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110334 --- Comment #10 from rguenther at suse dot de --- On Fri, 23 Jun 2023, hubicka at ucw dot cz wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110334 >=20 > --- Comment #8 from Jan Hubicka --- > > > I was playing with the idea of warning when at lto time when comdats = have > > > different command line options, but this triggers way too often in pr= actice. > >=20 > > Really? :/ > Yep, for example firefox consist of many extra libraries (skia, video > codecs, image format decoders...) each developed independently LTOed > into one libxul. Each of them has its own configure that tampers with > command line options. > These brings in libstdc++ comdats with different command line > sets (over 100 of different command lines for std::vector). >=20 > Firefox is bit extreme, but it is common for other bigger projects, too. > >=20 > > I think it would be desirable to diagnose these, maybe with an option to > > selectively disable this specific diagnostic. Because while it is not > > always a correctness issue it can be a performance issue as well. >=20 > I can dig out the patch then. But it was simply printing something like > warning: merging comdat function with function of same name but > different flags > note: first difference is -f..... > which was produced similar way we do diffs for optimization and target > attributes. >=20 > Now those -f.... were usually quite misleading as they tended to be > internal flags implied by something else (such as -Ofast instead of > -O2). Often the picked -f flag was obviously harmless and false > positive, however other -f flag might have been important. >=20 > So I concluded that it is not very easy information to interpret from user > perspective. >=20 > But indeed I agree htat this is not very obvious source of interesting > correctness & performance issues. Hmm, maybe we should then really go the semantic alias route for QOI reasons? We can then still diagnose "preserving comdat symbol because of different flags in TU X vs. Y" and maybe we can also estimate a total unit growth because of doing that? > >=20 > > Beware of new always-inline calls then appearing after greedy inlining > > (though that's exactly the case that we try to avoid here). I suppose > > you could disable inlining of a function which contains always-inline > > calls or simply functions that did not yet have the early inliner run > > on them (so keep the current behavior in cycles). Beware of indirect > > always-inline calls then. > >=20 > > Btw, for Skia the issue is really that some auto-generated CTOR isn't > > marked always-inline but everything else is. Maybe they should use > > flatten instead of always-inline ... >=20 > We disable inlining to always_inline during early inline, but not greedy > inline. Both of them can turn indirect calls to direct calls. So I was > thinking that as first cut we can inline only callees with no indirect > calls and no inlinable direct calls into always_inlines with no indirect > calls. > > Are you worried about possibility of early opt inventing new call into > builtin function that is defined as always_inline? No, just that we end up with a always-inline cycle (aka recursion) which we used to time/memory bomb on. That is, sth like the following has a "valid" inline solution (valid aka all always-inline calls are inlined), but only if we break the cycle at the 'forward' call. We used to mishandle this when unlucky with ordering (depending on which edge we enter the cycle when ordering for early opts). static void forward (void); static inline void __attribute__((always_inline)) always() { forward ();=20 } static void forward (void) { always (); }; void entry1 () { forward1 (); } void entry2 () { always (); } now you can make this less obvious a cycle via indirect calls, maybe not without taking the address of always-inline functions which would be on the border of being a user error. Richard.=