From patchwork Thu Feb 11 16:06:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 581945 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 1F26B14033B for ; Fri, 12 Feb 2016 03:06:50 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=sG6yQTDW; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=Ycwe2VBzN+b3arfJIbMdZAfdZWq93 xQ6z9ks1/X0SeZd4JmhOvdKvpRRiVWRzZsPYIXuUfMXuG7uGHsV8RjtpLEgthR5q o1ASGwCNJcEYiKeM9lgECqmATAwQBD3+UWNY8BY7SGKlIDD172nFUG3Mk7/mEuPB SUqR2aEdCjmODY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=2W+a9l1qFXbx5BZpJpgb3+Mt+ec=; b=sG6 yQTDW9pN3kXVDuCrKD3sIFrGewYwC9vVHjT3clVQLDShIFu56UGtQPJ2T2kTkAZq +EPvSQ4ZaiG8OeFBvAHr50syEn5zusm0rT1Ur4KLWhm5E3WT9+gmQ4WWBHWW7P8R RWbyGrFA9haVXvFGBR89u/5ruhbVk6COv0ikYbKg= Received: (qmail 79238 invoked by alias); 11 Feb 2016 16:06:42 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 79219 invoked by uid 89); 11 Feb 2016 16:06:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=iter, 1, 20, recreation, psi X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 11 Feb 2016 16:06:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 7D97DC0A5279; Thu, 11 Feb 2016 16:06:39 +0000 (UTC) Received: from tucnak.zalov.cz ([10.3.113.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1BG6bf4031927 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 11 Feb 2016 11:06:39 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id u1BG6ZjW005829; Thu, 11 Feb 2016 17:06:36 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id u1BG6X3V005828; Thu, 11 Feb 2016 17:06:33 +0100 Date: Thu, 11 Feb 2016 17:06:33 +0100 From: Jakub Jelinek To: Richard Biener , Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix ipa-split handling of clobbers and debug stmts in return_bb (PR ipa/68672) Message-ID: <20160211160633.GB3017@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes Hi! While the cgraph versioning code is able to deal with clobber stmts that refer to SSA_NAMEs set by basic blocks not split into the versioned function, and similarly with debug stmts (clobbers refererring to those are removed and debug stmts reset), on the main part that doesn't happen, because ipa-split makes the split blocks unreachable by jumping around them and thus if SSA_NAMEs set in the split basic blocks are referenced in return_bb in statements we intentionally ignored for the analysis (debug stmts and clobber stmts), we either end up with weird stuff (set debug stmts to debug temporary that is set in the basic blocks that are removed as dead), or for clobbers ICE. Richard has tried to deal with this by throwing away the return_bb in certain cases in the main part (forcing recreation of it), but that can throw valuable statements that we could have kept (debug stmts or e.g. decl clobbers or clobbers that don't need SSA_NAMEs from the split blocks), and doesn't work e.g. on the 1st and 3rd testcase below anyway. So, this patch reverts the main_part_return_p and instead does what is needed for those ignored stmts in return_bb: 1) SSA_NAME references to retval in both debug stmts and clobber stmts are replaced with the lhs of the call to *.part.* 2) debug stmts referencing other SSA_NAMEs set in the split bbs are reset 3) clobber stmts referencing other SSA_NAMEs set in the split bbs are removed The testcase hopefully cover all the interesting cases (return_bb copied from main part (where it is kept) to split part too, and with debug stmts and clobbers that refer to SSA_NAMEs set in main or split part or retval). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2016-02-11 Jakub Jelinek PR ipa/68672 * ipa-split.c (split_function): Don't compute/use main_part_return_p. Compute retval and retbnd early in all cases if split_part_return_p and return_bb is not EXIT. Remove all clobber stmts and reset all debug stmts that refer to SSA_NAMEs defined in split part, except if it is retval, in that case replace the old retval with the lhs of the call to the split part. * g++.dg/ipa/pr68672-1.C: New test. * g++.dg/ipa/pr68672-2.C: New test. * g++.dg/ipa/pr68672-3.C: New test. Jakub --- gcc/ipa-split.c.jj 2016-02-11 10:50:52.888220581 +0100 +++ gcc/ipa-split.c 2016-02-11 12:46:15.975777652 +0100 @@ -1244,28 +1244,13 @@ split_function (basic_block return_bb, s args_to_pass.safe_push (arg); } - /* See if the split function or the main part will return. */ - bool main_part_return_p = false; + /* See if the split function will return. */ bool split_part_return_p = false; FOR_EACH_EDGE (e, ei, return_bb->preds) { if (bitmap_bit_p (split_point->split_bbs, e->src->index)) split_part_return_p = true; - else - main_part_return_p = true; } - /* The main part also returns if we split on a fallthru edge - and the split part returns. */ - if (split_part_return_p) - FOR_EACH_EDGE (e, ei, split_point->entry_bb->preds) - { - if (! bitmap_bit_p (split_point->split_bbs, e->src->index) - && single_succ_p (e->src)) - { - main_part_return_p = true; - break; - } - } /* Add return block to what will become the split function. We do not return; no return block is needed. */ @@ -1279,8 +1264,8 @@ split_function (basic_block return_bb, s FIXME: Once we are able to change return type, we should change function to return void instead of just outputting function with undefined return value. For structures this affects quality of codegen. */ - else if (!split_point->split_part_set_retval - && (retval = find_retval (return_bb))) + else if ((retval = find_retval (return_bb)) + && !split_point->split_part_set_retval) { bool redirected = true; basic_block new_return_bb = create_basic_block (NULL, 0, return_bb); @@ -1308,12 +1293,10 @@ split_function (basic_block return_bb, s } /* When we pass around the value, use existing return block. */ else - bitmap_set_bit (split_point->split_bbs, return_bb->index); - - /* If the main part doesn't return pretend the return block wasn't - found for all of the following. */ - if (! main_part_return_p) - return_bb = EXIT_BLOCK_PTR_FOR_FN (cfun); + { + bitmap_set_bit (split_point->split_bbs, return_bb->index); + retbnd = find_retbnd (return_bb); + } /* If RETURN_BB has virtual operand PHIs, they must be removed and the virtual operand marked for renaming as we change the CFG in a way that @@ -1382,6 +1365,44 @@ split_function (basic_block return_bb, s DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0; } + /* If return_bb contains any clobbers that refer to SSA_NAMEs + set in the split part, remove them. Also reset debug stmts that + refer to SSA_NAMEs set in the split part. */ + if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun)) + { + gimple_stmt_iterator gsi = gsi_start_bb (return_bb); + while (!gsi_end_p (gsi)) + { + tree op; + ssa_op_iter iter; + gimple *stmt = gsi_stmt (gsi); + bool remove = false; + if (gimple_clobber_p (stmt) || is_gimple_debug (stmt)) + FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE) + { + basic_block bb = gimple_bb (SSA_NAME_DEF_STMT (op)); + if (op != retval + && bb + && bb != return_bb + && bitmap_bit_p (split_point->split_bbs, bb->index)) + { + if (is_gimple_debug (stmt)) + { + gimple_debug_bind_reset_value (stmt); + update_stmt (stmt); + } + else + remove = true; + break; + } + } + if (remove) + gsi_remove (&gsi, true); + else + gsi_next (&gsi); + } + } + /* If the original function is instrumented then it's part is also instrumented. */ if (with_bounds) @@ -1499,9 +1520,7 @@ split_function (basic_block return_bb, s return value into and put call just before it. */ if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun)) { - real_retval = retval = find_retval (return_bb); - retbnd = find_retbnd (return_bb); - + real_retval = retval; if (real_retval && split_point->split_part_set_retval) { gphi_iterator psi; @@ -1545,6 +1564,28 @@ split_function (basic_block return_bb, s break; } update_stmt (gsi_stmt (bsi)); + /* Also adjust clobbers and debug stmts in return_bb. */ + for (bsi = gsi_start_bb (return_bb); !gsi_end_p (bsi); + gsi_next (&bsi)) + { + gimple *stmt = gsi_stmt (bsi); + if (gimple_clobber_p (stmt) + || is_gimple_debug (stmt)) + { + ssa_op_iter iter; + use_operand_p use_p; + bool update = false; + FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, + SSA_OP_USE) + if (USE_FROM_PTR (use_p) == real_retval) + { + SET_USE (use_p, retval); + update = true; + } + if (update) + update_stmt (stmt); + } + } } /* Replace retbnd with new one. */ --- gcc/testsuite/g++.dg/ipa/pr68672-1.C.jj 2016-02-11 12:31:27.344497187 +0100 +++ gcc/testsuite/g++.dg/ipa/pr68672-1.C 2016-02-11 12:31:01.000000000 +0100 @@ -0,0 +1,20 @@ +// PR ipa/68672 +// { dg-do compile } +// { dg-options "-O -finline-small-functions -fpartial-inlining --param=partial-inlining-entry-probability=100" } + +void f2 (void *); +void *a; +struct C { virtual void m1 (); }; +struct D { C *m2 () { if (a) __builtin_abort (); } }; +D f1 (); +struct E { int e; ~E () { if (e) f2 (&e); } }; +E *b; +struct I { virtual void m3 (); }; + +void +I::m3 () +{ + if (a) + f1 ().m2 ()->m1 (); + b->~E (); +} --- gcc/testsuite/g++.dg/ipa/pr68672-2.C.jj 2016-02-11 12:33:50.744438948 +0100 +++ gcc/testsuite/g++.dg/ipa/pr68672-2.C 2016-02-11 12:32:50.000000000 +0100 @@ -0,0 +1,54 @@ +// PR ipa/68672 +// { dg-do compile } +// { dg-options "-O3 --param=partial-inlining-entry-probability=100 -g" } + +struct S { ~S () {} }; +S *a; +int *b; +void bar (); +void baz (); +void fn (int *); + +static int +foo () +{ + S *c = a; + if (c) + { + bar (); + if (a) + __builtin_abort (); + baz (); + } + int p = *b; + if (p) + { + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + } + c->~S (); + int q = 2 * p; + int r = 3 * q; + S *d = c; + return p; +} + +void +use1 () +{ + foo (); +} + +void +use2 () +{ + foo (); +} + +void +use3 () +{ + foo (); +} --- gcc/testsuite/g++.dg/ipa/pr68672-3.C.jj 2016-02-11 12:34:02.374272024 +0100 +++ gcc/testsuite/g++.dg/ipa/pr68672-3.C 2016-02-11 12:34:22.337985482 +0100 @@ -0,0 +1,57 @@ +// PR ipa/68672 +// { dg-do compile } +// { dg-options "-O3 --param=partial-inlining-entry-probability=100 -g" } + +struct S { ~S () {} }; +S *a, *e; +int *b; +void bar (); +void baz (); +void fn (int *); +void fn2 (S *); + +static int +foo () +{ + S *c = a; + if (c) + { + bar (); + if (a) + __builtin_abort (); + baz (); + } + int p = *b; + S *f = e; + if (p) + { + fn2 (f); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); fn (b); + } + f->~S (); + int q = 2 * p; + int r = 3 * q; + S *d = c; + return p; +} + +void +use1 () +{ + foo (); +} + +void +use2 () +{ + foo (); +} + +void +use3 () +{ + foo (); +}