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 <ja...@redhat.com>
>
> * 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 <x_7, ...>;
> + 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
