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 ESMTPS id 2CB4E385840A for ; Fri, 5 Nov 2021 20:45:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2CB4E385840A Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-451-2urN3JqXNJ6ct3QgMKpToA-1; Fri, 05 Nov 2021 16:45:01 -0400 X-MC-Unique: 2urN3JqXNJ6ct3QgMKpToA-1 Received: by mail-lf1-f70.google.com with SMTP id z1-20020a056512308100b003ff78e6402bso4113752lfd.4 for ; Fri, 05 Nov 2021 13:45:01 -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=kD/afxewU6omleQaxASbujlRXzEG30tG2f8e7DT5Ke4=; b=DwKKC9vbYlBLI6UCJXyhsgtPf0iP9b5CtrZGI2vXxjRvtzmAWUrlBDLq0DXW92Xq+p TWrwzptkDRb2vZrC+kiWcG7MzUUBHQwIIizadWqHB7oPOBKt6FiLw7OOq8zTRbDq5gcz DTZk8SJgvkdyD+tL4AQkIN0DQK6kP+PvQziWPLfWeOs/6WHPYbH6VJ204gIDPt79oVFg LtJj7s7lVKLygyWiKkPaAreIxgH0pSUbfr8lGJXrnNNc9csyhsD2PHeAPCTmilpSKsF2 v/Gal8BhdmBrZmaeC7MQu24JALMTkgjRScEz/qcCSZA0VXjmGFW8Ddg/8R9opM0aGspP K9bw== X-Gm-Message-State: AOAM533E25U6lXtBbCm08b3n3qY1evek/sAcYJpXOA/rJ4jcnZzmge5u XlEOJtsBPtb0DPuyUr0yWcsCLntBiByubTtFoNX76KFKX3UdD2qcKGnj7owxC/xn9Hm88VoYi4L uXlrP/2dCAQvaFFz+HfWN7oK5dI7WJdPt2Q== X-Received: by 2002:a2e:751a:: with SMTP id q26mr6723680ljc.168.1636145099953; Fri, 05 Nov 2021 13:44:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx/9hsU/ywHUbmmh5Gg6/fO4CebQKy2dcbtGRY5PZakDaX+OYr6gg6NlP73HSlpfE7OwI7BedKW7e3weSossg0= X-Received: by 2002:a2e:751a:: with SMTP id q26mr6723653ljc.168.1636145099670; Fri, 05 Nov 2021 13:44:59 -0700 (PDT) MIME-Version: 1.0 References: <20211105150905.37199-1-aldyh@redhat.com> <3d6f6538-e979-2440-18df-632296fb1abd@gmail.com> In-Reply-To: <3d6f6538-e979-2440-18df-632296fb1abd@gmail.com> From: Aldy Hernandez Date: Fri, 5 Nov 2021 21:44:48 +0100 Message-ID: Subject: Re: [PATCH] Cleanup back_threader::find_path_to_names. To: Jeff Law Cc: GCC patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="00000000000077d71705d010b78c" X-Spam-Status: No, score=-11.9 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_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 05 Nov 2021 20:45:04 -0000 --00000000000077d71705d010b78c Content-Type: text/plain; charset="UTF-8" On Fri, Nov 5, 2021 at 9:06 PM Jeff Law wrote: > > > > On 11/5/2021 9:09 AM, Aldy Hernandez wrote: > > The main path discovery function was due for a cleanup. First, > > there's a nagging goto and second, my bitmap use was sloppy. Hopefully > > this makes the code easier for others to read. > > > > Regstrapped on x86-64 Linux. I also made sure there were no difference > > in the number of threads with this patch. > > > > No functional changes. > > > > OK? > > > > gcc/ChangeLog: > > > > * tree-ssa-threadbackward.c (back_threader::find_paths_to_names): > > Remove gotos and other cleanups. > > --- > > gcc/tree-ssa-threadbackward.c | 52 ++++++++++++++--------------------- > > 1 file changed, 20 insertions(+), 32 deletions(-) > > > > diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c > > index b7eaff94567..d6a5b0b8da2 100644 > > --- a/gcc/tree-ssa-threadbackward.c > > +++ b/gcc/tree-ssa-threadbackward.c > > @@ -402,26 +402,18 @@ back_threader::find_paths_to_names (basic_block bb, bitmap interesting) > > > > m_path.safe_push (bb); > > > > + // Try to resolve the path without looking back. > > if (m_path.length () > 1 > > - && !m_profit.profitable_path_p (m_path, m_name, NULL)) > > + && (!m_profit.profitable_path_p (m_path, m_name, NULL) > > + || maybe_register_path ())) > > { > > m_path.pop (); > > m_visited_bbs.remove (bb); > > return false; > > } > > > > - // Try to resolve the path without looking back. > > - if (m_path.length () > 1 && maybe_register_path ()) > > - { > > - m_path.pop (); > > - m_visited_bbs.remove (bb); > > - return true; > > - } > Hmm, note the return values are different in these cases, which you've > merged to always return false. I was about to suggest you just kill > the return value and the cleanups that would enable. But I see your > patch uses the return value where we didn't before. Woah, good catch. It looks like the return value is no longer needed, so it can indeed be killed. This was left over from when resolve_phi() didn't resolve all incoming paths, but that's no longer the case. Once we exit find_path_to_names, all paths have been explored and there's nothing to do. OK pending tests? Aldy > > So I think that raises the question, for the recursive calls which set > "done", does the change in return value, particularly when > maybe_register_path returns true impact anything? > > jeff > --00000000000077d71705d010b78c Content-Type: text/x-patch; charset="US-ASCII"; name="0001-Cleanup-back_threader-find_path_to_names.patch" Content-Disposition: attachment; filename="0001-Cleanup-back_threader-find_path_to_names.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kvmuiskx0 RnJvbSAzMzI0OTVhYjZiZjM2NGJhYzU5ZDk2OGRmZGZlN2Q4ZDZhOWY1ZGNmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbGR5IEhlcm5hbmRleiA8YWxkeWhAcmVkaGF0LmNvbT4KRGF0 ZTogVGh1LCA0IE5vdiAyMDIxIDE5OjQ0OjE1ICswMTAwClN1YmplY3Q6IFtQQVRDSF0gQ2xlYW51 cCBiYWNrX3RocmVhZGVyOjpmaW5kX3BhdGhfdG9fbmFtZXMuCgpUaGUgbWFpbiBwYXRoIGRpc2Nv dmVyeSBmdW5jdGlvbiB3YXMgZHVlIGZvciBhIGNsZWFudXAuICBGaXJzdCwKdGhlcmUncyBhIG5h Z2dpbmcgZ290byBhbmQgc2Vjb25kLCBteSBiaXRtYXAgdXNlIHdhcyBzbG9wcHkuICBIb3BlZnVs bHkKdGhpcyBtYWtlcyB0aGUgY29kZSBlYXNpZXIgZm9yIG90aGVycyB0byByZWFkLgoKUmVnc3Ry YXBwZWQgb24geDg2LTY0IExpbnV4LiAgSSBhbHNvIG1hZGUgc3VyZSB0aGVyZSB3ZXJlIG5vIGRp ZmZlcmVuY2UKaW4gdGhlIG51bWJlciBvZiB0aHJlYWRzIHdpdGggdGhpcyBwYXRjaC4KCk5vIGZ1 bmN0aW9uYWwgY2hhbmdlcy4KCmdjYy9DaGFuZ2VMb2c6CgoJKiB0cmVlLXNzYS10aHJlYWRiYWNr d2FyZC5jIChiYWNrX3RocmVhZGVyOjpmaW5kX3BhdGhzX3RvX25hbWVzKToKCVJlbW92ZSBnb3Rv cyBhbmQgb3RoZXIgY2xlYW51cHMuCi0tLQogZ2NjL3RyZWUtc3NhLXRocmVhZGJhY2t3YXJkLmMg fCA2NSArKysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQs IDI0IGluc2VydGlvbnMoKyksIDQxIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2djYy90cmVl LXNzYS10aHJlYWRiYWNrd2FyZC5jIGIvZ2NjL3RyZWUtc3NhLXRocmVhZGJhY2t3YXJkLmMKaW5k ZXggYjdlYWZmOTQ1NjcuLjAwODVhZDAxY2RjIDEwMDY0NAotLS0gYS9nY2MvdHJlZS1zc2EtdGhy ZWFkYmFja3dhcmQuYworKysgYi9nY2MvdHJlZS1zc2EtdGhyZWFkYmFja3dhcmQuYwpAQCAtODks NyArODksNyBAQCBwcml2YXRlOgogICB2b2lkIGZpbmRfcGF0aHMgKGJhc2ljX2Jsb2NrIGJiLCB0 cmVlIG5hbWUpOwogICBib29sIGRlYnVnX2NvdW50ZXIgKCk7CiAgIGVkZ2UgbWF5YmVfcmVnaXN0 ZXJfcGF0aCAoKTsKLSAgYm9vbCBmaW5kX3BhdGhzX3RvX25hbWVzIChiYXNpY19ibG9jayBiYiwg Yml0bWFwIGltcG9ydHMpOworICB2b2lkIGZpbmRfcGF0aHNfdG9fbmFtZXMgKGJhc2ljX2Jsb2Nr IGJiLCBiaXRtYXAgaW1wb3J0cyk7CiAgIGJvb2wgcmVzb2x2ZV9kZWYgKHRyZWUgbmFtZSwgYml0 bWFwIGludGVyZXN0aW5nLCB2ZWM8dHJlZT4gJndvcmtsaXN0KTsKICAgdm9pZCByZXNvbHZlX3Bo aSAoZ3BoaSAqcGhpLCBiaXRtYXAgaW1wb3J0cyk7CiAgIGVkZ2UgZmluZF90YWtlbl9lZGdlIChj b25zdCB2ZWM8YmFzaWNfYmxvY2s+ICZwYXRoKTsKQEAgLTM4OCw0MCArMzg4LDI4IEBAIGJhY2tf dGhyZWFkZXI6OnJlc29sdmVfZGVmICh0cmVlIG5hbWUsIGJpdG1hcCBpbnRlcmVzdGluZywgdmVj PHRyZWU+ICZ3b3JrbGlzdCkKIC8vIEZpbmQganVtcCB0aHJlYWRpbmcgcGF0aHMgdG8gYW55IG9m IHRoZSBTU0EgbmFtZXMgaW4gdGhlCiAvLyBJTlRFUkVTVElORyBiaXRtYXAsIGFuZCByZWdpc3Rl ciBhbnkgc3VjaCBwYXRocy4KIC8vCi0vLyBSZXR1cm4gVFJVRSBpZiBubyBmdXJ0aGVyIHByb2Nl c3NpbmcgcGFzdCB0aGlzIGJsb2NrIGlzIG5lY2Vzc2FyeS4KLS8vIFRoaXMgaXMgYmVjYXVzZSB3 ZSd2ZSBlaXRoZXIgcmVnaXN0ZXJlZCBhIHBhdGgsIG9yIGJlY2F1c2UgdGhlcmUgaXMKLS8vIG5v dGhpbmcgb2YgaW50ZXJlc3RpbmcgYmV5b25kIHRoaXMgYmxvY2suCi0vLwogLy8gQkIgaXMgdGhl IGN1cnJlbnQgcGF0aCBiZWluZyBwcm9jZXNzZWQuCiAKLWJvb2wKK3ZvaWQKIGJhY2tfdGhyZWFk ZXI6OmZpbmRfcGF0aHNfdG9fbmFtZXMgKGJhc2ljX2Jsb2NrIGJiLCBiaXRtYXAgaW50ZXJlc3Rp bmcpCiB7CiAgIGlmIChtX3Zpc2l0ZWRfYmJzLmFkZCAoYmIpKQotICAgIHJldHVybiB0cnVlOwor ICAgIHJldHVybjsKIAogICBtX3BhdGguc2FmZV9wdXNoIChiYik7CiAKLSAgaWYgKG1fcGF0aC5s ZW5ndGggKCkgPiAxCi0gICAgICAmJiAhbV9wcm9maXQucHJvZml0YWJsZV9wYXRoX3AgKG1fcGF0 aCwgbV9uYW1lLCBOVUxMKSkKLSAgICB7Ci0gICAgICBtX3BhdGgucG9wICgpOwotICAgICAgbV92 aXNpdGVkX2Jicy5yZW1vdmUgKGJiKTsKLSAgICAgIHJldHVybiBmYWxzZTsKLSAgICB9Ci0KICAg Ly8gVHJ5IHRvIHJlc29sdmUgdGhlIHBhdGggd2l0aG91dCBsb29raW5nIGJhY2suCi0gIGlmICht X3BhdGgubGVuZ3RoICgpID4gMSAmJiBtYXliZV9yZWdpc3Rlcl9wYXRoICgpKQorICBpZiAobV9w YXRoLmxlbmd0aCAoKSA+IDEKKyAgICAgICYmICghbV9wcm9maXQucHJvZml0YWJsZV9wYXRoX3Ag KG1fcGF0aCwgbV9uYW1lLCBOVUxMKQorCSAgfHwgbWF5YmVfcmVnaXN0ZXJfcGF0aCAoKSkpCiAg ICAgewogICAgICAgbV9wYXRoLnBvcCAoKTsKICAgICAgIG1fdmlzaXRlZF9iYnMucmVtb3ZlIChi Yik7Ci0gICAgICByZXR1cm4gdHJ1ZTsKKyAgICAgIHJldHVybjsKICAgICB9CiAKICAgYXV0b19i aXRtYXAgcHJvY2Vzc2VkOwotICB1bnNpZ25lZCBpOwogICBib29sIGRvbmUgPSBmYWxzZTsKLQog ICAvLyBXZSB1c2UgYSB3b3JrbGlzdCBpbnN0ZWFkIG9mIGl0ZXJhdGluZyB0aHJvdWdoIHRoZSBi aXRtYXAsCiAgIC8vIGJlY2F1c2Ugd2UgbWF5IGFkZCBuZXcgaXRlbXMgaW4tZmxpZ2h0LgogICBh dXRvX3ZlYzx0cmVlPiB3b3JrbGlzdCAoYml0bWFwX2NvdW50X2JpdHMgKGludGVyZXN0aW5nKSk7 CkBAIC00MzMsMzcgKzQyMSwzMiBAQCBiYWNrX3RocmVhZGVyOjpmaW5kX3BhdGhzX3RvX25hbWVz IChiYXNpY19ibG9jayBiYiwgYml0bWFwIGludGVyZXN0aW5nKQogICAgICAgYmFzaWNfYmxvY2sg ZGVmX2JiID0gZ2ltcGxlX2JiIChTU0FfTkFNRV9ERUZfU1RNVCAobmFtZSkpOwogCiAgICAgICAv LyBQcm9jZXNzIGFueSBuYW1lcyBkZWZpbmVkIGluIHRoaXMgYmxvY2suCi0gICAgICBpZiAoZGVm X2JiID09IGJiKQorICAgICAgaWYgKGRlZl9iYiA9PSBiYgorCSAgJiYgYml0bWFwX3NldF9iaXQg KHByb2Nlc3NlZCwgaSkKKwkgICYmIHJlc29sdmVfZGVmIChuYW1lLCBpbnRlcmVzdGluZywgd29y a2xpc3QpKQogCXsKLQkgIGJpdG1hcF9zZXRfYml0IChwcm9jZXNzZWQsIGkpOwotCi0JICBpZiAo cmVzb2x2ZV9kZWYgKG5hbWUsIGludGVyZXN0aW5nLCB3b3JrbGlzdCkpCi0JICAgIHsKLQkgICAg ICBkb25lID0gdHJ1ZTsKLQkgICAgICBnb3RvIGxlYXZlX2JiOwotCSAgICB9CisJICBkb25lID0g dHJ1ZTsKKwkgIGJyZWFrOwogCX0KICAgICB9Ci0KICAgLy8gSWYgdGhlcmUgYXJlIGludGVyZXN0 aW5nIG5hbWVzIG5vdCB5ZXQgcHJvY2Vzc2VkLCBrZWVwIGxvb2tpbmcuCi0gIGJpdG1hcF9hbmRf Y29tcGxfaW50byAoaW50ZXJlc3RpbmcsIHByb2Nlc3NlZCk7Ci0gIGlmICghYml0bWFwX2VtcHR5 X3AgKGludGVyZXN0aW5nKSkKKyAgaWYgKCFkb25lKQogICAgIHsKLSAgICAgIGVkZ2VfaXRlcmF0 b3IgaXRlcjsKLSAgICAgIGVkZ2UgZTsKLSAgICAgIEZPUl9FQUNIX0VER0UgKGUsIGl0ZXIsIGJi LT5wcmVkcykKLQlpZiAoKGUtPmZsYWdzICYgRURHRV9BQk5PUk1BTCkgPT0gMCkKLQkgIGRvbmUg fD0gZmluZF9wYXRoc190b19uYW1lcyAoZS0+c3JjLCBpbnRlcmVzdGluZyk7CisgICAgICBiaXRt YXBfYW5kX2NvbXBsX2ludG8gKGludGVyZXN0aW5nLCBwcm9jZXNzZWQpOworICAgICAgaWYgKCFi aXRtYXBfZW1wdHlfcCAoaW50ZXJlc3RpbmcpKQorCXsKKwkgIGVkZ2VfaXRlcmF0b3IgaXRlcjsK KwkgIGVkZ2UgZTsKKwkgIEZPUl9FQUNIX0VER0UgKGUsIGl0ZXIsIGJiLT5wcmVkcykKKwkgICAg aWYgKChlLT5mbGFncyAmIEVER0VfQUJOT1JNQUwpID09IDApCisJICAgICAgZmluZF9wYXRoc190 b19uYW1lcyAoZS0+c3JjLCBpbnRlcmVzdGluZyk7CisJfQogICAgIH0KIAotIGxlYXZlX2JiOgot ICBiaXRtYXBfaXRlcmF0b3IgYmk7Ci0gIEVYRUNVVEVfSUZfU0VUX0lOX0JJVE1BUCAocHJvY2Vz c2VkLCAwLCBpLCBiaSkKLSAgICBiaXRtYXBfc2V0X2JpdCAoaW50ZXJlc3RpbmcsIGkpOwotCisg IC8vIFJlc2V0IHRoaW5ncyB0byB0aGVpciBvcmlnaW5hbCBzdGF0ZS4KKyAgYml0bWFwX2lvcl9p bnRvIChpbnRlcmVzdGluZywgcHJvY2Vzc2VkKTsKICAgbV9wYXRoLnBvcCAoKTsKICAgbV92aXNp dGVkX2Jicy5yZW1vdmUgKGJiKTsKLSAgcmV0dXJuIGRvbmU7CiB9CiAKIC8vIFNlYXJjaCBiYWNr d2FyZHMgZnJvbSBCQiBsb29raW5nIGZvciBwYXRocyB3aGVyZSB0aGUgZmluYWwKLS0gCjIuMzEu MQoK --00000000000077d71705d010b78c--