From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 1FA2A385801E; Thu, 4 Jan 2024 16:17:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1FA2A385801E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1704385053; bh=2wTQA0rsyJf4413LJv5O7x5GaB4SbKDVEBs+vGu6mJU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=q3z78SFU7jbLPX2QBjFDt5qZfdbSRgSNiJAceE9+KQtJcqisv6ZlyG9SBCjEa9p9U g+TGSnSDslST+abEO/N7SJIHoZcJfhjFpl+Jlx1Uhgzfv/LsZVBsILZiAuNJVK5Opa VTfKOROgQ4b1hWHIBqKKU40HJkWpl6P3idn4zxw4= From: "jakub at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/110852] [14 Regression] ICE: in get_predictor_value, at predict.cc:2695 with -O -fno-tree-fre and __builtin_expect() since r14-2219-geab57b825bcc35 Date: Thu, 04 Jan 2024 16:17:32 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: tree-optimization X-Bugzilla-Version: 14.0 X-Bugzilla-Keywords: ice-on-valid-code X-Bugzilla-Severity: normal X-Bugzilla-Who: jakub at gcc dot gnu.org 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=3D110852 --- Comment #10 from Jakub Jelinek --- (In reply to Jan Hubicka from comment #9) > By removing the logic we lose ability to optimize things like > a =3D b * c; > where b is predicted to value 0 and c has no useful prediction on it. No, that is what my unposted WIP patch did, but predict-18.c test catched t= hat. > > @@ -2631,6 +2623,9 @@ expr_expected_value_1 (tree type, tree o > >=20 > > if (predictor2 < *predictor) > > *predictor =3D predictor2; > > + if (*predictor !=3D PRED_BUILTIN_EXPECT > > + && *predictor !=3D PRED_BUILTIN_EXPECT_WITH_PROBABILITY) > > + *probability =3D -1; >=20 > This still can "upgrade" prediction to a predictor of lower enm value > but higher probability that is not conservative thing to do. > >=20 > > return res; > > } > I ended up with the folloing patch that also takes care of various cases > of phi merging and downgrading the predictor to new > PRED_COMBINED_VALUE_PREDICTION which can, like PRED_BUILTIN_EXPECT hold > custom probability but it is not trued as FIRST_MATCH. > What do you think? > + int p1 =3D get_predictor_value (*predictor, *probability); > + int p2 =3D get_predictor_value (predictor2, probability2); > + /* If both predictors agrees, it does not matter from which s/agrees/agree/ > + Consequently failing to fold both means that we will not suceed > determinging s/suceed/succeed/;s/determinging/determining/ Otherwise yes, but I think the code could be still simplified the way I had= in my patch (i.e. drop parts of the r14-2219 changes, and simply assume that failed recursion for one operand is PRED_UNCONDITIONAL instead of returning early, and not requiring the operands are INTEGER_CSTs, just that the resul= t of the binop folds to INTEGER_CST.=