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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 8FCFF383543D for ; Fri, 25 Jun 2021 16:20:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8FCFF383543D Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-335-_ABzeF0OP5CtqT95R4FUnA-1; Fri, 25 Jun 2021 12:20:11 -0400 X-MC-Unique: _ABzeF0OP5CtqT95R4FUnA-1 Received: by mail-wm1-f70.google.com with SMTP id v25-20020a1cf7190000b0290197a4be97b7so2882541wmh.9 for ; Fri, 25 Jun 2021 09:20:11 -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=oX+znyimi1AN/qnkysjXgJXZ5PPommRwEo/7RdLisAI=; b=DDdosTddmBxcR6BPug5AA2E1jE7IgW+3lP7ujGot2K1bPHn+jz3z0J7hTuLmzylaxS BsCZLDhRqhr6nsoWie0RK4Gc7xtsQ7IKof9lYbwM6q96xj6j29gNsudbRiTDazA4s7U6 rdywLtYD7ULbAlmJubCW2Ltmh1k+1ddGt1dpue3k/zuk6j3HoAbXTVSszIKgOHYpSS11 Wx+cWziqFgm7eScZVjOWDPAMFU9vRJll0Ak5pkZ391BQr35J7kSYRb+XMpsqIt2osSuW lLM/U8DWAhVZr9zuf4iyUUA1f3ZkcmngYHlkfOcod0ZDqf/v2tr9Kfa2uK5/rnpm8CkL M5/w== X-Gm-Message-State: AOAM530Yiz9HBwY8uxzbvrXdDwKKyhWMVdqwVeJ/x2BQUvdpA50TEMD0 A7hMqgmQxumP9rqzpjc/1oE9Kq/WgMKGt5z0v3GkwmFL/hHZ+bL0KZ+pnDCjiUIbR+B1PUB6KmT /l3kTPp4= X-Received: by 2002:a7b:ca48:: with SMTP id m8mr11312767wml.75.1624638010805; Fri, 25 Jun 2021 09:20:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwbp5lsoRiJag2sR4U8y9xVRKIBu9sKLzRbZip0xYfSvrlZpoF8PjMyuBHoprBEEZ5EQ2u/PQ== X-Received: by 2002:a7b:ca48:: with SMTP id m8mr11312736wml.75.1624638010478; Fri, 25 Jun 2021 09:20:10 -0700 (PDT) Received: from abulafia.quesejoda.com (128.red-81-33-16.staticip.rima-tde.net. [81.33.16.128]) by smtp.gmail.com with ESMTPSA id p13sm6294745wrt.89.2021.06.25.09.20.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 25 Jun 2021 09:20:10 -0700 (PDT) Subject: Re: replacing the backwards threader and more To: Richard Biener , Jeff Law Cc: GCC Mailing List , Andrew MacLeod , Martin Sebor References: <07775b9d-b8eb-48cb-57ef-9cc278d38967@redhat.com> <550e1ffd-957c-f348-49b6-b980c072c307@redhat.com> <3ac5e4e1-405f-fd5a-cb36-433a93f77df4@gmail.com> From: Aldy Hernandez Message-ID: <23a30c02-fa53-8734-88a8-72ec4d1e8211@redhat.com> Date: Fri, 25 Jun 2021 18:20:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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: Fri, 25 Jun 2021 16:20:15 -0000 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. 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. 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??). 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. 4. libphobos/src/std/net/isemail.d This is a D test where we don't actually fail, but we trigger the verification code. It is the only jump threading edge that the new code fails to get over the old code, and it only happens on ppc64. It triggers because a BB4 -> BB5 is too expensive to thread, but a BBn -> BB3 -> BB4 -> BB5 is considered safe to thread because BB3 is a latch and it alters the profitability equation. The reason we don't get it, is that we assume that if a X->Y is unprofitable, it is not worth looking at W->X->Y and so forth. Jeff had some fancy ideas on how to attack this. Once such idea was to stop looking back, but only for things we were absolutely sure would never yield a profitable path. I tried a subset of this, by allowing further looks on this latch test, but my 1.55% overall performance penalty turned into an 8.33% penalty. Personally it looks way too expensive for this one isolated case. Besides, the test where this clamping code originally came from still succeeds (commit eab2541b860c48203115ac6dca3284e982015d2c). CONCLUSION ========== That's basically it. If we agree the above things are not big issues, or can be addressed as follow-ups, I'd like to start the ball rolling on the new threader. This would allow more extensive testing of the code, and separate it a bit from the other big changes coming up :). Aldy