Re: [PATCH][middle-end/88784] Middle end is missing some optimizations about unsigned

[Richard Biener <rguenther at suse dot de>]

On Tue, 16 Jul 2019, Li Jia He wrote:

> Hi,
> 
>   I made some changes based on the recommendations. Would you like to
>   help me to see it again ? Sorry, it took so long time to provide the
>   patch.
> 
>   Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
> 	The reason is that I did not found a good way to handle the
> 	optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
> 	Maybe I missing some important information about match.pd.
> 	2. The gimple_resimplify2 function is not used.  Since stmt1,
> 	stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
> 	question with the value on the stack as the return value ?
> 	I may have misunderstood Richard's intention.

Sorry for the delay in reviewing.

Rather than exporting gimple_set_code and gimple_size I'd split
out a

void
gimple_init (gimple *, enum gimple_code code, unsigned nops);

from gimple_alloc (changing that to GC allocate not cleared
memory) doing all of the actual initialization.  Then the
allocation would go like

gimple *stmt1 = (gimple *)XALLOCAVEC (char, gimple_size (GIMPLE_ASSIGN, 
3));
gimple_init (stmt1, GIMPLE_ASSIGN, 3);

with an overload for gimple_size to account for # of ops.

+  /* Allocate SSA names(lhs1) on the stack.  */
+  tree lhs1 = XALLOCA (tree_node);

you can use (tree) XALLOCA (tree_ssa_name) here

+  /* Call the interface function of match.pd to simplify the expression.  
*/
+  tree t = gimple_simplify (code, boolean_type_node, gimple_assign_lhs 
(stmt1),
+                           gimple_assign_lhs (stmt2), NULL,
+                           follow_all_ssa_edges);
+
+  if (t)

As I told Martin offline the appropriate function to use is

 You'd do

  gimple_match_op op (gimple_match_cond::UNCOND, code,
         boolean_type_node, gimple_assign_lhs (stmt1),
gimple_assign_lhs (stmt2));
  if (op->resimplify (NULL, follow_all_ssa_edges))
   {
      if (gimple_simplified_result_is_gimple_val (res_op))
        .. got a constant or SSA name ..
      else if (res_op->code.is_tree_code ()
                 && TREE_CODE_CLASS ((tree_code)res_op->code)) ==
tcc_comparison)
        ... got a comparison res_op->op[0] res_op->code res_op->op[1] ...

so you get the outermost expression back decomposed.

Otherwise with you passing NULL as the gimple_seq you'll miss quite
some simplification cases.

You also have to watch out for the result containing the LHS of one
of your temporary stmts.

+  if (tree t = maybe_fold_comparisons_from_match_pd (BIT_AND_EXPR, code1, 
op1a,
+                                                    op1b, code2, op2a, 
op2b))
+    return t;
+
+  if (tree t = maybe_fold_comparisons_from_match_pd (BIT_AND_EXPR, code2, 
op2a,
+                                                    op2b, code1, op1a, 
op1b))
+    return t;

with match.pd rules you shouldn't need to call this twice, once with
swapped operands.

Otherwise this first patch looks like what I'd have done and we
can build upon it.

Not sure if you or Martin wants to improve it according to my
comments.

Thanks,
Richard.