From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A179D3858429 for ; Fri, 29 Jul 2022 11:43:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A179D3858429 Received: from mail-ot1-f69.google.com (mail-ot1-f69.google.com [209.85.210.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-352-4019XLALO3m0ryTxtWfzVA-1; Fri, 29 Jul 2022 07:43:24 -0400 X-MC-Unique: 4019XLALO3m0ryTxtWfzVA-1 Received: by mail-ot1-f69.google.com with SMTP id e14-20020a9d63ce000000b0061c6ca80c54so1783187otl.0 for ; Fri, 29 Jul 2022 04:43:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rLAQMzBrdr5D+HCqRvClg40VVI2X4UMe/s4RqsDV6Fo=; b=vphIMCfqAnmsmz4nx6xtQs405HkR8oz/XpTewYbBE0vCA6wXkX0fU8OK8wjOQtY4Gf uK0JU6VWD3rrSYuZmMhrHzLgDVgxPgLjvvqTm7k/AG4Z++VRqWf9ppg4AOQiwZ7T+HtZ GEMm7faMwfmVkWOZT2UHanx7DN2GaSoxj//RsO4Mv18io0f9UorEExSWWjhJDRwEwQti aX67vPvQ5LVHWOp67aXo7MIKyCrBzGQwGtId1VUpT1E83+eH7KyeHX8oNYSDePMhrK8r HDIn/ylMqINRbxc3YAdHiAd070M5NXc6Gq3v3KqYnwULJLyxrFCpMMqvOlaj0pUkWHcJ cncA== X-Gm-Message-State: AJIora/CJN6QtZwbi+70NSGwEzGPeZNsI0XOgHFzMHiHoLkci+1qPSyk a8kFzED5wT0CP6pTYEObONMWkRmBNvzv75gUuNLooRfwPK538EZm2hfk59bxi+oasdPia7PuqU2 dcgoL9mzXKOJv0PF6bOjsNWF5W3F6tEPnuw== X-Received: by 2002:a4a:88cf:0:b0:435:bf6f:dff0 with SMTP id q15-20020a4a88cf000000b00435bf6fdff0mr1107968ooh.30.1659095003378; Fri, 29 Jul 2022 04:43:23 -0700 (PDT) X-Google-Smtp-Source: AGRyM1svbgWVJqZoKd6ItqXGiwOqK5oMZ8gNgBE+68/ZVI+ajsqWea5lWhz0NQlh2qWylph0MCdOzshnw0j60o78Eww= X-Received: by 2002:a4a:88cf:0:b0:435:bf6f:dff0 with SMTP id q15-20020a4a88cf000000b00435bf6fdff0mr1107962ooh.30.1659095003158; Fri, 29 Jul 2022 04:43:23 -0700 (PDT) MIME-Version: 1.0 References: <20220729085417.4B50F13A1B@imap2.suse-dmz.suse.de> In-Reply-To: <20220729085417.4B50F13A1B@imap2.suse-dmz.suse.de> From: Aldy Hernandez Date: Fri, 29 Jul 2022 13:43:12 +0200 Message-ID: Subject: Re: [PATCH] tree-optimization/105679 - disable backward threading of unlikely entry To: Richard Biener Cc: gcc-patches , "MacLeod, Andrew" , Jeff Law X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 11:43:26 -0000 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? 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 >