From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 26304384D169; Thu, 15 Feb 2024 14:55:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 26304384D169 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1708008925; bh=MwnMT0r2xZbCzVKCQsR0IAyqvuiJHtVYetgcaUu6ZVs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=OqZzu0yGjn8RMLydtdqQR12wg5rNeIvS5n81RRt71383d2APh8q3m9l7jIyM4aeFP iH3u8ZA5Zbz7cm22uSZ/gUWF4fnP64s0KS+NXqTmMB0xJ5zt9AaPhQd8nBDU2TNPPZ t2OM21BEZf9JZSnycUlfX8xkdvmAYRNHYoFqSEyc= From: "hubicka at ucw dot cz" To: gcc-bugs@gcc.gnu.org Subject: [Bug middle-end/113907] [14 regression] ICU miscompiled since on x86 since r14-5109-ga291237b628f41 Date: Thu, 15 Feb 2024 14:55:22 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: middle-end X-Bugzilla-Version: 14.0 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: hubicka at ucw dot cz X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: 14.0 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=3D113907 --- Comment #36 from Jan Hubicka --- > > Having a testcase is great. I was just playing with crafting one. > > I am still concerned about value ranges in ipa-prop's jump functions. >=20 > Maybe my imagination is too limited, but if the ipa-prop's jump functions= are > computed before ICF is performed, then if you call function a which makes= some > assumptions about its arguments and SSA_NAMEs used within the function and > results in some return range > and another one which makes different assumptions and results in a differ= ent > range, > then even if the two functions are merged and either the range info is th= rown > away > or unioned, I think if some functions call one or the other functions then > still the original range assumptions still apply, depending on which one > actually called. I think the tescase should have functions A1 and A2 calling function B. We need to arrange A1 to ICF into A2 and make A2 to have narrower value range of parameter of B visible to ipa-prop. Since ICF happens first, ipa-prop will not see A1's value range and specialize B for A2's range. Which sould make it possible to trigger wrong code. >=20 > > Let me see if I can modify the testcase to also trigger problem with va= lue > > ranges in ipa-prop jump functions. > >=20 > > Not streaming value ranges is an omission on my side (I mistakely assum= ed we > > do stream them). We ought to stream them, since otherwise we will lose > > propagated return value ranges in partitioned programs, which is pity. >=20 > Not sure if it is a good idea to start streaming them now in stage4, but = sure, > if we > stream them (and I think we should mostly have code to be able to stream = that It probably makes sense to postpone VR stremaing for stage1. (Even thouh it is not hard to do.) I will add that to my TODO. > because we can stream the ranges in the jump functions, just unsure about > points-to stuff), > then I still think best approach would be to merge regardless of range in= fo, > but in that case preferrably union instead of throw away. The only quest= ion is > where to do the merging for the LTO case (where to stream the union of the > ranges and where to read it in and update for the SSA_NAMEs). With LTO ICF streams in all function bodies for comparsion to WPA and keeps the winning one, so it is about extending the comparator to keep track of difference of value ranges for all compared pairs. There is code to merge profiles, so at that place one can also do other kind of metadata merging. ICF needs a lot of love on this - value range merging is just one example. We also want to merge TBAA (so we can merge different template instatiations) and other stuff. At the moment we handle all other metadat conservatively (i.e. do not attempt to merge, just refuse merging if it differs) so value ranges are first that are handled aggressively with your patch. I think it is fine, since most value range will be recomputed in late optimization - the only exceptions are the return value ranges for now. I will try to work on making this more systematic next stage1.=