From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id CD5FA3858429 for ; Fri, 29 Jul 2022 12:21:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD5FA3858429 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id C5BC420EAC; Fri, 29 Jul 2022 12:21:51 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id A77E52C141; Fri, 29 Jul 2022 12:21:51 +0000 (UTC) Date: Fri, 29 Jul 2022 12:21:51 +0000 (UTC) From: Richard Biener To: Aldy Hernandez cc: gcc-patches , "MacLeod, Andrew" , Jeff Law Subject: Re: [PATCH] tree-optimization/105679 - disable backward threading of unlikely entry In-Reply-To: Message-ID: References: <20220729085417.4B50F13A1B@imap2.suse-dmz.suse.de> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2022 12:21:55 -0000 On Fri, 29 Jul 2022, Aldy Hernandez wrote: > On Fri, Jul 29, 2022 at 11:02 AM Richard Biener wrote: > > > > The following makes the backward threader reject threads whose entry > > edge is probably never executed according to the profile. That in > > particular, for the testcase, avoids threading the irq == 1 check > > on the path where irq > 31, thereby avoiding spurious -Warray-bounds > > diagnostics > > > > if (irq_1(D) > 31) > > goto ; [0.00%] > > else > > goto ; [100.00%] > > > > ;; basic block 3, loop depth 0, count 0 (precise), probably never executed > > _2 = (unsigned long) irq_1(D); > > __builtin___ubsan_handle_shift_out_of_bounds (&*.Lubsan_data0, 1, _2); > > > > _3 = 1 << irq_1(D); > > mask_4 = (u32) _3; > > entry = instance_5(D)->array[irq_1(D)]; > > capture (mask_4); > > if (level_6(D) != 0) > > goto ; [34.00%] > > else > > goto ; [66.00%] > > > > ;; basic block 5, loop depth 0, count 708669600 (estimated locally), maybe hot if (irq_1(D) == 1) > > goto ; [20.97%] > > else > > goto ; [79.03%] > > > > Bootstrap and regtest running on x86_64-unknown-linux-gnu. > > > > The testcase in the PR requries both ubsan and sancov so I'm not sure > > where to put it but IIRC there were quite some duplicate PRs wrt > > threading unlikely paths exposing diagnostics, eventually some > > testcase will come out of those (when we identify them). Note > > the patch is quite conservative in only disabling likely never > > executed paths rather than requiring maybe_hot_edge_p (OTOH those > > are somewhat similar in the end). > > Sounds reasonable, if for no other reason than to quiet down the > annoyingly large amount of false positives we have because of > aggressive threading, or better ranges as a whole. > > How does this fit in with Jeff's original idea of isolating known > problematic paths (null dereferences?), which in theory are also > unlikely? If profile estimation gets to see those the paths should turn unlikely. Path isolation should also make sure to adjust the profile accordingly, though local profile adjustments are somewhat difficult. Richard. > Thanks for doing this. > Aldy > > > > > I'm going to push it when testing finishes but maybe there are some > > testcases to adjust. > > > > PR tree-optimization/105679 > > * tree-ssa-threadbackwards.cc > > (back_threader_profitability::profitable_path_p): Avoid threading > > when the entry edge is probably never executed. > > --- > > gcc/tree-ssa-threadbackward.cc | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/gcc/tree-ssa-threadbackward.cc b/gcc/tree-ssa-threadbackward.cc > > index 3519aca84cd..90f5331c265 100644 > > --- a/gcc/tree-ssa-threadbackward.cc > > +++ b/gcc/tree-ssa-threadbackward.cc > > @@ -777,6 +777,15 @@ back_threader_profitability::profitable_path_p (const vec &m_path, > > "exceeds PARAM_MAX_FSM_THREAD_PATH_INSNS.\n"); > > return false; > > } > > + edge entry = find_edge (m_path[m_path.length () - 1], > > + m_path[m_path.length () - 2]); > > + if (probably_never_executed_edge_p (cfun, entry)) > > + { > > + if (dump_file && (dump_flags & TDF_DETAILS)) > > + fprintf (dump_file, " FAIL: Jump-thread path not considered: " > > + "path entry is probably never executed.\n"); > > + return false; > > + } > > } > > else if (!m_speed_p && n_insns > 1) > > { > > -- > > 2.35.3 > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)