Re: [PATCH] Handling == or != comparisons that may affect range test optimization.

[Jeff Law]

On 10/31/13 18:03, Cong Hou wrote:
(This patch is for the bug 58728:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58728)
As in the bug report, consider the following loop:

int foo(unsigned int n)
{
   if (n != 0)
   if (n != 1)
   if (n != 2)
   if (n != 3)
   if (n != 4)
     return ++n;
   return n;
}

The range test optimization should be able to merge all those five
conditions into one in reassoc pass, but I fails to do so. The reason
is that the phi arg of n is replaced by the constant it compares to in
case of == or != comparisons (in vrp pass). GCC checks there is no
side effect on n between any two neighboring conditions by examining
if they defined the same phi arg in the join node. But as the phi arg
is replace by a constant, the check fails.

This patch deals with this situation by considering the existence of
== or != comparisons, which is attached below (a text file is also
attached with proper tabs). Bootstrap and make check both get passed.

Any comment?

+       bool is_eq_expr = is_cond && (gimple_cond_code (stmt) == NE_EXPR
+                                       || gimple_cond_code (stmt) == EQ_EXPR)
+                                 && TREE_CODE (phi_arg) == INTEGER_CST;
+
+       if (is_eq_expr)
+         {
+           lhs = gimple_cond_lhs (stmt);
+           rhs = gimple_cond_rhs (stmt);
+
+           if (operand_equal_p (lhs, phi_arg, 0))
+             {
+               tree t = lhs;
+               lhs = rhs;
+               rhs = t;
+             }
+           if (operand_equal_p (rhs, phi_arg, 0)
+               && operand_equal_p (lhs, phi_arg2, 0))
+             continue;
+         }
+
+       gimple stmt2 = last_stmt (test_bb);
+       bool is_eq_expr2 = gimple_code (stmt2) == GIMPLE_COND
+                            && (gimple_cond_code (stmt2) == NE_EXPR
+                                || gimple_cond_code (stmt2) == EQ_EXPR)
+                            && TREE_CODE (phi_arg2) == INTEGER_CST;
+
+       if (is_eq_expr2)
+         {
+           lhs2 = gimple_cond_lhs (stmt2);
+           rhs2 = gimple_cond_rhs (stmt2);
+
+           if (operand_equal_p (lhs2, phi_arg2, 0))
+             {
+               tree t = lhs2;
+               lhs2 = rhs2;
+               rhs2 = t;
+             }
+           if (operand_equal_p (rhs2, phi_arg2, 0)
+               && operand_equal_p (lhs2, phi_arg, 0))
+             continue;
+         }

Can you factor those two hunks of nearly identical code into a single 
function and call it twice?  I'm also curious if you really need the 
code to swap lhs/rhs.  When can the LHS of a cond be an integer 
constant?  Don't we canonicalize it as <ssa_name> <COND> <constant>?

I'd probably write the ChangeLog as:

        * tree-ssa-reassoc.c (suitable_cond_bb): Handle constant PHI
        operands resulting from propagation of edge equivalences.


I'm also curious -- did this code show up in a particular benchmark, if 
so, which one?

jeff