From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 412DB3858C62; Sun, 15 Oct 2023 01:16:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 412DB3858C62 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1697332584; bh=YiHYvDTKlmBNqMSBDCa6Oo8uewyz/BcRERQ8665MapQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=r2aIrCNSw4syEt1zY6MpZ6+HNg3L2k785Cm58gCV15B1wlZ9zteswiiUTJcsNBURs qsViix3rH+JM4ddavJz+7efT+HgfOC8Jn3G4sU3QrsH690aSc5rYpUPN6DF9jS/tsV 9b/v4twHeKYtzOoWOsmvwP4OrDORw6t4uqpFohAk= From: "danglin at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug regression/111709] [13/14 Regression] Miscompilation of sysdeps/ieee754/dbl-64/s_fma.c Date: Sun, 15 Oct 2023 01:15:50 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: regression X-Bugzilla-Version: 13.1.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: danglin at gcc dot gnu.org 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.3 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: cc 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=3D111709 John David Anglin changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aldyh at redhat dot com, | |jeffreyalaw at gmail dot c= om --- Comment #12 from John David Anglin --- The miscompilation of s_fma.c was introduced by the following change: dave@atlas:~/gnu/gcc/gcc$ git bisect good 8c99e307b20c502e55c425897fb3884ba8f05882 is the first bad commit commit 8c99e307b20c502e55c425897fb3884ba8f05882 Author: Aldy Hernandez Date: Sat Jun 25 18:58:02 2022 -0400 Convert DOM to use Ranger rather than EVRP [Jeff, this is the same patch I sent you last week with minor tweaks to the commit message.] [Despite the verbosity of the message, this is actually a pretty straightforward patch. It should've gone in last cycle, but there was a nagging regression I couldn't get to until after stage1 had closed.] There are 3 uses of EVRP in DOM that must be converted. Unfortunately, they need to be converted in one go, so further splitting of this patch would be problematic. There's nothing here earth shattering. It's all pretty obvious in retrospect, but I've added a short description of each use to aid in reviewing: * Convert evrp use in cprop to ranger. This is easy, as cprop in DOM was converted to the ranger API last cycle, so this is just a matter of using a ranger instead of an evrp_range_analyzer. * Convert evrp use in threader to ranger. The idea here is to use the hybrid approach we used for the initial VRP threader conversion last cycle. The DOM threader will continue using the forward threader infrastructure while continuing to query DOM data structures, and only if the conditional does not relsolve, using the ranger. This gives us the best of both worlds, and is a proven approach. Furthermore, as frange and prange come live in the next cycle, we can move away from the forward threader altogether, and just add another backward threader. This will not only remove the last use of the forward threader, but will allow us to remove at least 1 or 2 threader instances. * Convert conditional folding to use the method used by the ranger and evrp. Previously DOM was calling into the guts of simplify_using_ranges::vrp_visit_cond_stmt. The blessed way now is using fold_cond() which rewrites the conditional and edges automatically. When legacy is removed, simplify_using_ranges will be further cleaned up, and there will only be one entry point into simplifying a statement. * DOM was setting global ranges determined from unreachable edges as a side-effect of using the evrp engine. We must handle these cases before nuking evrp, and DOM seems like a good fit. I've just moved the snippet to DOM, but it could live anywhere else we do a DOM walk. For the record, this is the case *vrp handled: : ... if (c_5(D) !=3D 5) goto ; else goto ; : __builtin_unreachable (); : If M dominates all uses of c_5, we can set the global range of c_5 to [5,5]. I have tested on x86-64, pcc64le, and aarch64 Linux. I also ran threading benchmarks as well as performance benchmarks. DOM threads 1.56% more paths which ultimately yields a miniscule total increase of 0.03%. The conversion to ranger brings a 7.87% performance drop in DOM, which is a wash in overall compilation. This is in line with other replacements of legacy evrp with ranger. We handle a lot more cases. It's not free . There is a a regression on Wstringop-overflow-4.C which I'm planning on XFAILing. It's another variant of the usual middle-end false positives: having no ranges produces no warnings, but slightly refined ranges, or worse-- isolating specific problematic cases in the threader causes flare-ups. As an aside, as Richi has suggested, I think we should discuss restricting the threader's ability to thread highly unlikely paths. These cause no end of pain for middle-end warnings. However, I don't know if this would conflict with path isolation for things like null dereferencing. ISTR you were interested in this. BTW, I think the Wstringop-overflow-4.C test is problematic and I've attached my analysis. Basically the regression is caused by a bad interaction with the rounding/alignment that placement new has inlined into the IL. This happens for int16_r[] which the test is testing. Ranger can glean some range info, which causes DOM threading to isolate a path which causes a warning. OK for trunk? gcc/ChangeLog: * tree-ssa-dom.cc (dom_jt_state): Pass ranger to constructor instead of evrp. (dom_jt_state::push): Remove m_evrp. (dom_jt_state::pop): Same. (dom_jt_state::record_ranges_from_stmt): Remove. (dom_jt_state::register_equiv): Remove updating of evrp ranges. (class dom_jt_simplifier): Pass ranger to constructor. Inherit from hybrid_jt_simplifier. (dom_jt_simplifier::simplify): Convert to ranger. (pass_dominator::execute): Same. (all_uses_feed_or_dominated_by_stmt): New. (dom_opt_dom_walker::set_global_ranges_from_unreachable_edges): New. (dom_opt_dom_walker::before_dom_children): Call set_global_ranges_from_unreachable_edges. Do not call record_ranges_from_stmt. (dom_opt_dom_walker::after_dom_children): Remove evrp use. (cprop_operand): Use int_range<> instead of value_range. (dom_opt_dom_walker::fold_cond): New. (dom_opt_dom_walker::optimize_stmt): Pass ranger to cprop_into_stmt. Use fold_cond() instead of vrp_visit_cond_stmt(). * tree-ssa-threadedge.cc (jt_state::register_equivs_stmt): Do n= ot pass state to simplifier. * vr-values.h (class vr_values): Make fold_cond public. gcc/testsuite/ChangeLog: * gcc.dg/sancov/cmp0.c: Adjust for conversion to ranger. * gcc.dg/tree-ssa/ssa-dom-branch-1.c: Same. * gcc.dg/tree-ssa/ssa-dom-thread-7.c: Same. * gcc.dg/vect/bb-slp-pr81635-2.c: Same. * gcc.dg/vect/bb-slp-pr81635-4.c: Same. * g++.dg/warn/Wstringop-overflow-4.C: Likewise. * gcc.target/mips/data-sym-multi-pool.c: Likewise. * gcc.target/mips/mips.exp: Likewise. gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C | 34 ++++ gcc/testsuite/gcc.dg/sancov/cmp0.c | 2 +- gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-branch-1.c | 5 +- gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c | 2 +- gcc/testsuite/gcc.dg/vect/bb-slp-pr81635-2.c | 2 +- gcc/testsuite/gcc.dg/vect/bb-slp-pr81635-4.c | 6 +- .../gcc.target/mips/data-sym-multi-pool.c | 2 +- gcc/testsuite/gcc.target/mips/mips.exp | 1 + gcc/tree-ssa-dom.cc | 223 +++++++++++------= ---- gcc/tree-ssa-threadedge.cc | 4 +- gcc/vr-values.h | 2 +- 11 files changed, 170 insertions(+), 113 deletions(-) I don't know anything about ranger but I wonder if this has to do with the mips/hppa NaN representation.=