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 0BFBC385E007 for ; Mon, 28 Jun 2021 22:29:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0BFBC385E007 Received: by mail-ot1-x32f.google.com with SMTP id m6-20020a9d1d060000b029044e2d8e855eso18596123otm.8 for ; Mon, 28 Jun 2021 15:29:19 -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=nScUs+XkjoWbsSAW+ooEE/uNsCA58uM6+1amRHSLNZE=; b=EUNHGYnDJG+CB8rfin3/bFIl9s+knnDQqAgNrL3eBjvVQxNhL4ObNhHjOZmLY9woFn H1PfDaFOcNXCZfYAT+slRkQCuVkrF2dM9QMCfN4RiQn7zVF1d+DTfl+pLE/C19psQMO/ C3ATBDHOXhl7zEXfyY1Xl/lAkx1YKHsqIbzy4uhiZiO/CHgEN9Ivfh6snKQHRgHntOD5 TyGrahpw48OfR2DS3U53+qEhGYZordjpWWNFNCOfLMK7zA8IdajJ6f380CCYzS21dW9k DADQWgBP2/dvwTXofRoFvV9c3SP91IdCAlOr9Lag1pxPe1tYOWWf6zP+4MDoujd1l9zY qY1Q== X-Gm-Message-State: AOAM533vENF5ctPhjEDbeO5yXypGr9gNXehF64dzyPzfpFbOd5Edm/6G hLn+iHCiKG2A+F92EdXAxWw= X-Google-Smtp-Source: ABdhPJzZXlEDeY+4ySmB4wCrDNsvCR5NFEf+qF9O49Ag/9JPn8SVaI9EDuRtg1T+wjIgI1u20Zqk/A== X-Received: by 2002:a9d:4d8c:: with SMTP id u12mr1590520otk.67.1624919358515; Mon, 28 Jun 2021 15:29:18 -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 m11sm2560949otp.29.2021.06.28.15.29.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Jun 2021 15:29:18 -0700 (PDT) Subject: Re: replacing the backwards threader and more To: Aldy Hernandez , Richard Biener , Jeff Law Cc: GCC Mailing List , Martin Sebor 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> From: Martin Sebor Message-ID: <5150f538-2f38-42c6-5c1e-8ced525cd909@gmail.com> Date: Mon, 28 Jun 2021 16:29:17 -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: <06474b9b-b770-9308-fd02-f07a118bcb6b@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.0 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: Mon, 28 Jun 2021 22:29:20 -0000 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 >