[PATCH] Improve detection of constant conditions during jump threading

[Jeff Law]

On 04/19/2016 11:50 AM, Patrick Palka wrote:

> 1. This patch introduces a "regression" in gcc.dg/tree-ssa/ssa-thread-11.c
> in that we no longer perform FSM threading during vrp2 but instead we
> detect two new jump threading opportunities during vrp1.  Not sure if
> the new code is better but it is shorter.  I wonder how this should be
> resolved...
Definitely not a regression.  As you note we thread two jumps earlier 
utilizing your new code.  With the old code we're dependent upon other 
simplifications occurring which eventually exposes the FSM threads that 
the test is checking for in vrp2.

I think we just want to remove the test for FSM jump threads in VRP2. 
We get coverage for your test via ssa-thread-14.  That should just leave 
the verification that we do not have irreducible loops at the end of 
VRP2 in ssa-thread-11.  I'll make that change.

I do see what appears to be a missed jump thread, but that's not 
affected positively or negatively by your change.  There's a reasonable 
chance it's only exposed by normal thread jumps in VRP2 and since we 
don't iterate, it's left in the IL.  I haven't analyzed it in detail, 
but my hope is that when I pull the backwards threading out into its own 
pass that we'll start picking up more of these secondary effects.


>
> gcc/ChangeLog:
>
> 	* tree-ssa-threadedge.c (simplify_control_stmt_condition): Split
> 	out into ...
> 	(simplify_control_stmt_condition_1): ... here.  Recurse into
> 	BIT_AND_EXPRs and BIT_IOR_EXPRs.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/tree-ssa/ssa-thread-14.c: New test.
> ---
I fixed a few formatting nits (too long lines).


> +
> +	  if (res1 != NULL_TREE && res2 != NULL_TREE)
> +	    {
> +	      if (rhs_code == BIT_AND_EXPR
> +		  && TYPE_PRECISION (TREE_TYPE (op0)) == 1
> +		  && integer_nonzerop (res1)
> +		  && integer_nonzerop (res2))
> +		{
> +		  /* If A != 0 and B != 0 then (bool)(A | B) != 0 is true.  */
> +		  if (cond_code == NE_EXPR)
> +		    return one_cst;
> +		  /* If A != 0 and B != 0 then (bool)(A | B) == 0 is false.  */
> +		  if (cond_code == EQ_EXPR)
> +		    return zero_cst;
I think you wanted (A & B) in the two immediately preceding comments, 
which I fixed.


I suspect there's other stuff we could do in this space, but you've 
probably covered the most important cases.

> +      /* Handle (A CMP B) CMP 0.  */
> +      else if (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
> +	       == tcc_comparison)
> +	{
> +	  tree rhs1 = gimple_assign_rhs1 (def_stmt);
> +	  tree rhs2 = gimple_assign_rhs2 (def_stmt);
> +
> +	  tree_code new_cond = gimple_assign_rhs_code (def_stmt);
> +	  if (cond_code == EQ_EXPR)
> +	    new_cond = invert_tree_comparison (new_cond, false);
> +
> +	  tree res
> +	    = simplify_control_stmt_condition_1 (e, def_stmt, avail_exprs_stack,
> +						 rhs1, new_cond, rhs2,
> +						 dummy_cond, simplify,
> +						 handle_dominating_asserts,
> +						 limit - 1);
> +	  if (res != NULL_TREE && is_gimple_min_invariant (res))
> +	    return res;
I was a bit confused by this case, but then realized that we already 
narrowed COND_CODE to EQ_EXPR/NE_EXPR.

I made the minor edits noted above and committed the change.

Thanks,
Jeff