From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128297 invoked by alias); 4 Mar 2015 13:30:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 128288 invoked by uid 89); 4 Mar 2015 13:30:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-oi0-f50.google.com Received: from mail-oi0-f50.google.com (HELO mail-oi0-f50.google.com) (209.85.218.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 04 Mar 2015 13:30:14 +0000 Received: by oigh136 with SMTP id h136so5540482oig.1 for ; Wed, 04 Mar 2015 05:30:12 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.202.204.88 with SMTP id c85mr2708786oig.81.1425475812656; Wed, 04 Mar 2015 05:30:12 -0800 (PST) Received: by 10.76.98.137 with HTTP; Wed, 4 Mar 2015 05:30:12 -0800 (PST) In-Reply-To: References: <54F3F648.8090400@redhat.com> <54F697A5.6090908@redhat.com> Date: Wed, 04 Mar 2015 13:30:00 -0000 Message-ID: Subject: Re: [patch/committed] PR middle-end/65233 make walk-ssa_copies handle empty PHIs From: Richard Biener To: Jeff Law Cc: Aldy Hernandez , gcc-patches , Jan Hubicka Content-Type: multipart/mixed; boundary=001a1134fd4c741fe805107673a8 X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00211.txt.bz2 --001a1134fd4c741fe805107673a8 Content-Type: text/plain; charset=UTF-8 Content-length: 3268 On Wed, Mar 4, 2015 at 1:41 PM, Richard Biener wrote: > On Wed, Mar 4, 2015 at 6:27 AM, Jeff Law wrote: >> On 03/02/15 01:38, Richard Biener wrote: >>> >>> On Mon, Mar 2, 2015 at 6:34 AM, Aldy Hernandez wrote: >>>> >>>> As I mention in the PR... >>>> >>>> What's happening here is that the ipa_polymorphic_call_context >>>> constructor >>>> is calling walk_ssa_copies on a PHI node that has no arguments. This >>>> happens because finalize_jump_threads eventually removes some PHI >>>> arguments >>>> as it's redirecting some edges, leaving a PHI with no arguments: >>>> >>>> SR.33_23 = PHI <> >>>> >>>> This should get cleaned up later, but the IPA polymorphic code gets >>>> called >>>> during the actual CFG clean-up, and walk_ssa_copies cannot handle an >>>> empty >>>> PHI. >>>> >>>> Approved by Honza. >>>> >>>> Fully tested on x86-64 Linux and verified that the patch fixes the ICE on >>>> an >>>> x86-64 Linux cross aarch64-linux-gnu cc1plus. >>>> >>>> Committed to mainline. >>> >>> >>> I think the real issue is that the walking code is executed via fold_stmt >>> when >>> called with an API that tells you not to walk SSA use-def chains. >> >> ? We have something that tells us not to walk the chains? I don't see it >> in an API for fold_stmt. How is the ipa-polymorphic code supposed to know >> when it can't follow the chains? > > It gets passed the valueize callback now which returns NULL_TREE for > SSA names we can't follow. Btw, for match-and-simplify I had to use that as default for fold_stmt _exactly_ because of the call to fold_stmt from replace_uses_by via merge-blocks from cfgcleanup. This is because replace-uses-by doesn't have all uses replaced before it folds the stmt! We also have the "weaker" in-place flag. >> The restrictions on what we can do while we're in the inconsistent state >> prior to updating the ssa graph aren't defined anywhere and I doubt anyone >> really knows what they are. That's obviously concerning. >> >> We might consider trying to narrow the window in which these inconsistencies >> are allowed. To do that I think we need to split cfgcleanup into two >> distinct parts. First is unreachable block removal (which is needed so that >> we can compute the dominators). Second is everything else. >> >> The order of operations would be something like >> >> remove unreachable blocks >> ssa graph update >> rest of cfg_cleanup >> >> That just feels too intrusive to try at this stage though. > > Well, not folding statements from cfg-cleanup would be better. > > I'll have a look at the testcase in the PR and will come back with a > suggestion on what to do for GCC 5. I'd say that the devirtualization code is quite a heavy thing do to from fold_stmt. Yes - it want's to catch all cases if a stmt is modified (after which passes should fold it). So I am testing the following on x86_64 (verified it fixes the testcase with a aarch64 cross). Richard. 2015-03-04 Richard Biener PR middle-end/65233 * ipa-polymorphic-call.c: Include tree-ssa-operands.h and tree-into-ssa.h. (walk_ssa_copies): Revert last chage. Instead do not walk SSA names registered for SSA update. --001a1134fd4c741fe805107673a8 Content-Type: application/octet-stream; name=fix-pr65233 Content-Disposition: attachment; filename=fix-pr65233 Content-Transfer-Encoding: base64 X-Attachment-Id: f_i6urhpwx0 Content-length: 2245 MjAxNS0wMy0wNCAgUmljaGFyZCBCaWVuZXIgIDxyZ3VlbnRoZXJAc3VzZS5k ZT4KCglQUiBtaWRkbGUtZW5kLzY1MjMzCgkqIGlwYS1wb2x5bW9ycGhpYy1j YWxsLmM6IEluY2x1ZGUgdHJlZS1zc2Etb3BlcmFuZHMuaCBhbmQKCXRyZWUt aW50by1zc2EuaC4KCSh3YWxrX3NzYV9jb3BpZXMpOiBSZXZlcnQgbGFzdCBj aGFnZS4gIEluc3RlYWQgZG8gbm90IHdhbGsKCVNTQSBuYW1lcyByZWdpc3Rl cmVkIGZvciBTU0EgdXBkYXRlLgoKSW5kZXg6IGdjYy9pcGEtcG9seW1vcnBo aWMtY2FsbC5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIGdjYy9pcGEt cG9seW1vcnBoaWMtY2FsbC5jCShyZXZpc2lvbiAyMjExNDkpCisrKyBnY2Mv aXBhLXBvbHltb3JwaGljLWNhbGwuYwkod29ya2luZyBjb3B5KQpAQCAtODEs NiArODEsOCBAQCBhbG9uZyB3aXRoIEdDQzsgc2VlIHRoZSBmaWxlIENPUFlJ TkczLgogI2luY2x1ZGUgImRhdGEtc3RyZWFtZXIuaCIKICNpbmNsdWRlICJs dG8tc3RyZWFtZXIuaCIKICNpbmNsdWRlICJzdHJlYW1lci1ob29rcy5oIgor I2luY2x1ZGUgInRyZWUtc3NhLW9wZXJhbmRzLmgiCisjaW5jbHVkZSAidHJl ZS1pbnRvLXNzYS5oIgogCiAvKiBSZXR1cm4gdHJ1ZSB3aGVuIFRZUEUgY29u dGFpbnMgYW4gcG9seW1vcnBoaWMgdHlwZSBhbmQgdGh1cyBpcyBpbnRlcmVz dGluZwogICAgZm9yIGRldmlydHVhbGl6YXRpb24gbWFjaGluZXJ5LiAgKi8K QEAgLTgwNCw3ICs4MDYsOSBAQCB3YWxrX3NzYV9jb3BpZXMgKHRyZWUgb3As IGhhc2hfc2V0PHRyZWU+CiAgIFNUUklQX05PUFMgKG9wKTsKICAgd2hpbGUg KFRSRUVfQ09ERSAob3ApID09IFNTQV9OQU1FCiAJICYmICFTU0FfTkFNRV9J U19ERUZBVUxUX0RFRiAob3ApCi0JICYmIFNTQV9OQU1FX0RFRl9TVE1UIChv cCkKKwkgLyogV2UgbWlnaHQgYmUgY2FsbGVkIHZpYSBmb2xkX3N0bXQgZHVy aW5nIGNmZ2NsZWFudXAgd2hlcmUKKwkgICAgU1NBIGZvcm0gbmVlZCBub3Qg YmUgdXAtdG8tZGF0ZS4gICovCisJICYmICFuYW1lX3JlZ2lzdGVyZWRfZm9y X3VwZGF0ZV9wIChvcCkgCiAJICYmIChnaW1wbGVfYXNzaWduX3NpbmdsZV9w IChTU0FfTkFNRV9ERUZfU1RNVCAob3ApKQogCSAgICAgfHwgZ2ltcGxlX2Nv ZGUgKFNTQV9OQU1FX0RFRl9TVE1UIChvcCkpID09IEdJTVBMRV9QSEkpKQog ICAgIHsKQEAgLTgzNSwxMCArODM5LDcgQEAgd2Fsa19zc2FfY29waWVzICh0 cmVlIG9wLCBoYXNoX3NldDx0cmVlPgogCXsKIAkgIGdpbXBsZSBwaGkgPSBT U0FfTkFNRV9ERUZfU1RNVCAob3ApOwogCi0JICBpZiAoZ2ltcGxlX3BoaV9u dW1fYXJncyAocGhpKSA+IDIKLQkgICAgICAvKiBXZSBjYW4gYmUgY2FsbGVk IHdoaWxlIGNsZWFuaW5nIHVwIHRoZSBDRkcgYW5kIGNhbgotCQkgaGF2ZSBl bXB0eSBQSElzIGFib3V0IHRvIGJlIHJlbW92ZWQuICAqLwotCSAgICAgIHx8 IGdpbXBsZV9waGlfbnVtX2FyZ3MgKHBoaSkgPT0gMCkKKwkgIGlmIChnaW1w bGVfcGhpX251bV9hcmdzIChwaGkpID4gMikKIAkgICAgZ290byBkb25lOwog CSAgaWYgKGdpbXBsZV9waGlfbnVtX2FyZ3MgKHBoaSkgPT0gMSkKIAkgICAg b3AgPSBnaW1wbGVfcGhpX2FyZ19kZWYgKHBoaSwgMCk7Cg== --001a1134fd4c741fe805107673a8--