From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12052 invoked by alias); 29 Oct 2013 11:28:53 -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 12038 invoked by uid 89); 29 Oct 2013 11:28:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Oct 2013 11:28:52 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id DA882A5ACE; Tue, 29 Oct 2013 12:28:49 +0100 (CET) Date: Tue, 29 Oct 2013 12:05:00 -0000 From: Richard Biener To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Handle __builtin_unreachable () using assertions in VRP In-Reply-To: <20131025090640.GF30970@tucnak.zalov.cz> Message-ID: References: <20131025090640.GF30970@tucnak.zalov.cz> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2013-10/txt/msg02392.txt.bz2 On Fri, 25 Oct 2013, Jakub Jelinek wrote: > Hi! > > As discussed on IRC, this patch attempts to preserve VRP computed > range info for some simple __builtin_unreachable () using assertions. > If there are no immediate uses of some SSA_NAME except for those in > a condition guarding __builtin_unreachable () and in ASSERT_EXPR > in the following basic block, we can copy the range info from the assert > lhs (which is lost during remove_range_assertions) and thus preserve it. > Eventually we might then remove the __builtin_unreachable () in that case > and still benefit from user annotating the source. > In the testcase below in bar we have even without this patch the expected > range info on the SSA_NAME set to x + 1, but in foo there was no range > info preserved. As we have just one user of get_range_info right now, > not creating a testcase. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > int > foo (int x) > { > if (x < 26 || x > 37) > __builtin_unreachable (); > return x; > } > > int > bar (int x) > { > if (x < 26 || x > 37) > __builtin_unreachable (); > return x + 1; > } > > 2013-10-25 Jakub Jelinek > > * tree-vrp.c (remove_range_assertions): If ASSERT_EXPR_VAR > has no other immediate uses but in the condition and ASSERT_EXPR > and the other successor of the predecessor bb is > __builtin_unreachable (), set_range_info of the ASSERT_EXPR_VAR > to the range info of the ASSERT_EXPR's lhs. > > --- gcc/tree-vrp.c.jj 2013-10-24 10:19:21.000000000 +0200 > +++ gcc/tree-vrp.c 2013-10-24 14:32:29.065878208 +0200 > @@ -6488,12 +6488,16 @@ remove_range_assertions (void) > { > basic_block bb; > gimple_stmt_iterator si; > + /* 1 if looking at ASSERT_EXPRs immediately at the beginning of > + a basic block preceeded by GIMPLE_COND branching to it and > + __builtin_trap, -1 if not yet checked, 0 otherwise. */ > + int is_unreachable; > > /* Note that the BSI iterator bump happens at the bottom of the > loop and no bump is necessary if we're removing the statement > referenced by the current BSI. */ > FOR_EACH_BB (bb) > - for (si = gsi_start_bb (bb); !gsi_end_p (si);) > + for (si = gsi_after_labels (bb), is_unreachable = -1; !gsi_end_p (si);) > { > gimple stmt = gsi_stmt (si); > gimple use_stmt; > @@ -6501,30 +6505,96 @@ remove_range_assertions (void) > if (is_gimple_assign (stmt) > && gimple_assign_rhs_code (stmt) == ASSERT_EXPR) > { > + tree lhs = gimple_assign_lhs (stmt); > tree rhs = gimple_assign_rhs1 (stmt); > tree var; > tree cond = fold (ASSERT_EXPR_COND (rhs)); > - use_operand_p use_p; > + use_operand_p use_p, use2_p; > imm_use_iterator iter; > > gcc_assert (cond != boolean_false_node); > > - /* Propagate the RHS into every use of the LHS. */ > var = ASSERT_EXPR_VAR (rhs); > - FOR_EACH_IMM_USE_STMT (use_stmt, iter, > - gimple_assign_lhs (stmt)) > + gcc_assert (TREE_CODE (var) == SSA_NAME); > + > + if (!POINTER_TYPE_P (TREE_TYPE (lhs)) > + && SSA_NAME_RANGE_INFO (lhs)) > + { > + if (is_unreachable == -1) > + { > + is_unreachable = 0; > + if (single_pred_p (bb)) > + { > + basic_block pred_bb = single_pred (bb); > + gimple last = last_stmt (pred_bb); > + if (last && gimple_code (last) == GIMPLE_COND) > + { > + basic_block other_bb > + = EDGE_SUCC (pred_bb, 0)->dest; > + if (other_bb == bb) > + other_bb = EDGE_SUCC (pred_bb, 1)->dest; > + if (EDGE_COUNT (other_bb->succs) == 0) > + { > + gimple_stmt_iterator gsi > + = gsi_after_labels (other_bb); > + if (!gsi_end_p (gsi) > + && gimple_call_builtin_p > + (gsi_stmt (gsi), > + BUILT_IN_UNREACHABLE)) > + is_unreachable = 1; > + } > + } Please factor this out into a separate function. Like obfuscated_fallthru_edge_p () (better name?). Best somewhere in tree-cfg.c. Also you want to skip debug stmts ... (not sure if we reliably enough remove them, but I can envision a compare-debug fail with some DCE disabled at least). > + } > + } > + /* Handle > + if (x_7 >= 10 && x_7 < 20) > + __builtin_unreachable (); > + x_8 = ASSERT_EXPR ; > + if the only uses of x_7 are in the ASSERT_EXPR and > + in the condition. In that case, we can copy the > + range info from x_8 computed in this pass also > + for x_7. */ > + if (is_unreachable) > + { > + bool ok = true; > + FOR_EACH_IMM_USE_FAST (use_p, iter, var) > + if (USE_STMT (use_p) != stmt) > + { > + use_stmt = USE_STMT (use_p); > + if (is_gimple_debug (use_stmt)) > + continue; > + while (is_gimple_assign (use_stmt) > + && single_imm_use > + (gimple_assign_lhs (use_stmt), > + &use2_p, &use_stmt)) > + ; > + if (gimple_code (use_stmt) != GIMPLE_COND > + || gimple_bb (use_stmt) != single_pred (bb)) > + { > + ok = false; > + break; > + } > + } This also should be split out. Otherwise ok. Thanks, Richard. > + if (ok) > + set_range_info (var, SSA_NAME_RANGE_INFO (lhs)->min, > + SSA_NAME_RANGE_INFO (lhs)->max); > + } > + } > + > + /* Propagate the RHS into every use of the LHS. */ > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs) > FOR_EACH_IMM_USE_ON_STMT (use_p, iter) > - { > - SET_USE (use_p, var); > - gcc_assert (TREE_CODE (var) == SSA_NAME); > - } > + SET_USE (use_p, var); > > /* And finally, remove the copy, it is not needed. */ > gsi_remove (&si, true); > release_defs (stmt); > } > else > - gsi_next (&si); > + { > + gsi_next (&si); > + is_unreachable = 0; > + } > } > } > > > Jakub