From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32f.google.com (mail-ot1-x32f.google.com [IPv6:2607:f8b0:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id 596F5385800F for ; Tue, 29 Jun 2021 22:16:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 596F5385800F Received: by mail-ot1-x32f.google.com with SMTP id 110-20020a9d0a770000b0290466fa79d098so419893otg.9 for ; Tue, 29 Jun 2021 15:16:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=XN2olpiB50OG0faJRVIrUIgZrtBo1QKA0g76hrCsRSo=; b=fpTsj/GeZpMdAs61MRbkLfykb/lAleC6ZvgMsGdiSZRDx70gftmYbI9yHq21JKWEwI xxynwC6ISXFi6+lMFb2X3bUe6RWVV4hlmeZmqfU88gwPmaBPI0jJZX+v+IS76Bjhs9/O 4ltRfcSvVRN2fQaRFCTXXVAbhQ0BtsQ1ll4+AmgoSCW6aTA8S3Tl5+ESPvkK3+2boz2w m+jinswHRInhOvSsbnOvs/TV+1j/6zcgDubgI7en4BGrEsSd9cOWUP4Xm30O5H5JcdtT HhjqbnueGJhJjTA42IcZ7Gp3ZNcScFwyPgoj6CA0gScHfITGgea5m5q7Wqy0XeDhC8pJ Dfog== X-Gm-Message-State: AOAM532O17RcRTinmJaYtMhxTrTq/ZcHyHBa/N4/1l3uC7zAZMKghFB8 lvc63sNzjX70ec2TdVF68UQZoHaMaiYDSQ== X-Google-Smtp-Source: ABdhPJzJEsDBCBa+yWIYqcX4fbjcdtAuCuivSAoJzAbeowLDF+0j/JVpMWNgDSSQnlBO+OU2vAsfqg== X-Received: by 2002:a9d:4592:: with SMTP id x18mr6368016ote.74.1625005003545; Tue, 29 Jun 2021 15:16:43 -0700 (PDT) Received: from [192.168.0.41] (97-118-105-195.hlrn.qwest.net. [97.118.105.195]) by smtp.gmail.com with ESMTPSA id r14sm4074847oie.43.2021.06.29.15.16.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Jun 2021 15:16:43 -0700 (PDT) Subject: Re: replacing the backwards threader and more To: Aldy Hernandez Cc: Richard Biener , Jeff Law , GCC Mailing List References: <07775b9d-b8eb-48cb-57ef-9cc278d38967@redhat.com> <550e1ffd-957c-f348-49b6-b980c072c307@redhat.com> <3ac5e4e1-405f-fd5a-cb36-433a93f77df4@gmail.com> <23a30c02-fa53-8734-88a8-72ec4d1e8211@redhat.com> <06474b9b-b770-9308-fd02-f07a118bcb6b@redhat.com> <5150f538-2f38-42c6-5c1e-8ced525cd909@gmail.com> From: Martin Sebor Message-ID: <78952609-a961-3867-36c1-fcc669b2da8e@gmail.com> Date: Tue, 29 Jun 2021 16:16:39 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jun 2021 22:16:46 -0000 On 6/28/21 6:32 PM, Aldy Hernandez wrote: > I had changed my approach to passing -Wno-free-nonheap-object in the > Makefile. Can you try disabling the Makefile bit and bootstrapping, > cause that was still failing. I see. I just tested it and my patch does let the existing #pragma suppress the warning. I just pinged the core patch yesterday so once it and the rest of the series gets approved there will be no need for the Makefile workaround. Martin > > Thanks. > Aldy > > On Tue, Jun 29, 2021, 00:29 Martin Sebor > wrote: > > On 6/28/21 12:17 AM, Aldy Hernandez wrote: > > > > > > On 6/25/21 7:19 PM, Martin Sebor wrote: > >> On 6/25/21 10:20 AM, Aldy Hernandez via Gcc wrote: > >>> Hi folks. > >>> > >>> I'm done with benchmarking, testing and cleanups, so I'd like > to post > >>> my patchset for review.  However, before doing so, I'd like to > >>> address a handful of meta-issues that may affect how I post these > >>> patches. > >>> > >>> Trapping on differences > >>> ======================= > >>> > >>> Originally I wanted to contribute verification code that would > trap > >>> if the legacy code threaded any edges the new code couldn't (to be > >>> removed after a week).  However, after having tested on various > >>> architectures and only running once into a missing thread, I'm > >>> leaning towards omitting the verification code, since it's > fragile, > >>> time consuming, and quite hacky. > >>> > >>> For the record, I have tested on x86-64, aarch64, ppc64 and > ppc64le. > >>> There is only one case, across bootstrap and regression tests > where > >>> the verification code is ever tripped (discussed below). > >>> > >>> Performance > >>> =========== > >>> > >>> I re-ran benchmarks as per our callgrind suite, and the penalty > with > >>> the current pipeline is 1.55% of overall compilation time.  As is > >>> being discussed, we should be able to mitigate this > significantly by > >>> removing other threading passes. > >>> > >>> Failing testcases > >>> ================= > >>> > >>> I have yet to run into incorrect code being generated, but I > have had > >>> to tweak a considerable number of tests.  I have verified every > >>> single discrepancy and documented my changes in the testsuite > when it > >>> merited doing so.  However, there are a couple tests that trigger > >>> regressions and I'd like to ask for guidance on how to address > them. > >>> > >>> 1. gcc.c-torture/compile/pr83510.c > >>> > >>> I would like to XFAIL this. > >>> > >>> What happens here is that thread1 threads a switch statement such > >>> that the various cases have been split into different independent > >>> blocks. One of these blocks exposes an arr[i_27] access which is > >>> later propagated by VRP to be arr[10].  This is an invalid access, > >>> but the array bounds code doesn't know it is an unreachable path. > >> > >> The test has a bunch of loops that iterate over the 10 array > elements. > >> There have been bug reports about loop unrolling causing false > positives > >> -Warray-bounds (e.g., PR 92539, 92110, or 86341) so this could be > >> the same issue. > >> > >>> > >>> However, it is not until dom2 that we "know" that the value of the > >>> switch index is such that the path to arr[10] is unreachable.  For > >>> that matter, it is not until dom3 that we remove the > unreachable path. > >> > >> If you do XFAIL it can you please isolate a small test case and open > >> a bug and make it a -Warray-bounds blocker? > > > > Will do. > > > >> > >>> > >>> 2. -Wfree-nonheap-object > >>> > >>> This warning is triggered while cleaning up an auto_vec.  I see > that > >>> the va_heap::release() inline is wrapped with a pragma ignore > >>> "-Wfree-nonheap-object", but this is not sufficient because jump > >>> threading may alter uses in such a way that > may_emit_free_warning() > >>> will warn on the *inlined* location, thus bypassing the pragma. > >>> > >>> I worked around this with a mere: > >>> > >>>  > @@ -13839,6 +13839,7 @@ maybe_emit_free_warning (tree exp) > >>>>    location_t loc = tree_inlined_location (exp); > >>>> +  loc = EXPR_LOCATION (exp); > >>> > >>> but this causes a ton of Wfree-nonheap* tests to fail.  I think > >>> someone more knowledgeable should address this (msebor??). > >> > >> This sounds like the same problem as PR 98871.  Does the patch below > >> fix it? > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572515.html > >> If so, I suggest getting that patch in first to avoid testsuite > >> failures.  If it doesn't fix it I'll look into it before you commit > >> your changes. > > > > The latest patch in the above thread does not apply.  Do you have > a more > > recent patch? > > The first patch in the series applies cleanly for me: > https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572516.html > but patch 2/4 does need a few adjustments.  With those and your > latest changes applied I don't see any -Wfree-nonheap-object test > failures. > > Martin > > > > >>> 3. uninit-pred-9_b.c > >>> > >>> The uninit code is getting confused with the threading and the > bogus > >>> warning in line 24 is back.  I looked at the thread, and it is > correct. > >>> > >>> I'm afraid all these warnings are quite fragile in the presence of > >>> more aggressive optimizations, and I suspect it will only get > worse. > >> > >>  From my recent review of open -Wmaybe-uninitialized bugs (and > >> the code) it does seem to be both fragile and getting worse.  I've > >> only found a few simple problems so far in the code but nothing that > >> would make a dramatic difference so I can't say if it's possible to > >> do much better, but I'm not done or ready to give up.  If you XFAIL > >> this too please open a bug for it and make it a blocker for > >> -Wuninitialized? > > > > Will do. > > > > Good to know these are somewhat known issues. > > > > Thanks. > > Aldy > > >