https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45397

--- Comment #26 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 17 Feb 2017, law at redhat dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45397
> 
> --- Comment #25 from Jeffrey A. Law <law at redhat dot com> ---
> "When doing so allows for simplification" is actually a pretty low bar here. 
> We just have to prove the widening cast is a common subexpression.  Once we do
> that, we know widening is a win.  And that's relatively easy to discover.  You
> go back to the SSA_NAME_DEF_STMT of the object you want to widen, then walk
> over its immediate uses to see if one is a proper widening op that dominates
> the expression we're trying to simplify. 
> 
> That's it, nothing else needed to prove profitability.  We only get ~20 hits 
> in
> a bootstrap, but I didn't expect this idiom to show up all that much.

I find this kind of "technique" of finding CSE opportunities quite ugly
and I'm not sure where you'd actually implement this...

> 
> --
> 
> Trying to do all the pattern matching in phi-opt and ultimately generate the
> min/max is going to be *hideous* because we're trying to do too  many things 
> at
> once.  We have components of CSE, VRP and DCE that we'd have to bring into
> phi-opt, which seems wrong to me.

Well, we are pattern matching the overflow builtins as well.  I think
it would be quite natural to pattern match saturating operations with
the premise to generate better code for them (either by generic expansion
to min/max or by utilizing CPU instructions -- I think SSE has saturating
ops for example).  You don't really need CSE, VRP and DCE, you "simply"
pattern match the comparisons.  You'd then replace the PHI node by
the saturated op and let DCE do the rest for you.

> Contrast that to simplifying the IL like my match.pd pattern does.   We make a
> simple, profitable, change to the IL.  That one profitable change in the IL 
> has
> ripple effects that allow subsequent passes to do their jobs with the final
> result that the minmax detection code is presented with exactly the IL it 
> knows
> how to transform.
> 
> --
> 
> Alternately (and I haven't prototyped this) we could try again to look at this
> as a redundancy elimination problem.
> 
> Given X = A + B in DOM, where A comes from a narrowed operand (A').  Lookup a
> widened B in the expression table (B').  If found, then lookup A' + B'.  If
> found (lhs), then replace X = A + B with X = (typeof X) ((lhs) & mask).
> 
> That's essentially doing the same thing as the prototype match.pd pattern. 
> Except that we know the operand widening as well as the widened arithmetic are
> both CSEs.

CSE already does similar stuff for loads, transparently adding 
VIEW_CONVERT_EXPRs.  I've added generic helpers for this
(vn_nary_build_or_lookup).  The question is if it's worth it.
You'd basically add to the list of special patterns in visit_nary_op
where we already handle ops in different sign (you'd add "op in different
width" and possibly add two stmts instead of one - the conversion and
the masking).

Oh, happens to be a patch in my local tree only (no longer remember
what was the reason to invent it but surely the question is how
many of these special-cases are worth the extra lookups)

Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c        (revision 245417)
+++ gcc/tree-ssa-sccvn.c        (working copy)
@@ -3453,17 +3493,78 @@ visit_copy (tree lhs, tree rhs)
 static bool
 visit_nary_op (tree lhs, gimple *stmt)
 {
-  bool changed = false;
   tree result = vn_nary_op_lookup_stmt (stmt, NULL);
-
   if (result)
-    changed = set_ssa_val_to (lhs, result);
-  else
-    {
-      changed = set_ssa_val_to (lhs, lhs);
-      vn_nary_op_insert_stmt (stmt, lhs);
+    return set_ssa_val_to (lhs, result);
+  else if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+          && (TYPE_PRECISION (TREE_TYPE (lhs))
+              == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (lhs))))
+          && is_gimple_assign (stmt))
+    {
+      /* For operations on integers that have the same result independent
+         of the signedness of their operands try to lookup a result with
+        opposite sign.  */
+      enum tree_code code = gimple_assign_rhs_code (stmt);
+      switch (code)
+       {
+       case NEGATE_EXPR:
+         {
+           tree type = TREE_TYPE (lhs);
+           type = signed_or_unsigned_type_for (! TYPE_UNSIGNED (type), 
type);
+           tree ops[3];
+           ops[0] = gimple_assign_rhs1 (stmt);
+           ops[0] = vn_nary_op_lookup_pieces (1, NOP_EXPR, type, ops, 
NULL);
+           if (ops[0])
+             {
+               ops[0] = vn_nary_op_lookup_pieces (1, code, type,
+                                                  ops, NULL);
+               if (ops[0])
+                 {
+                   result = vn_nary_build_or_lookup (NOP_EXPR,
+                                                     TREE_TYPE (lhs), 
ops);
+                   if (result)
+                     return set_ssa_val_to (lhs, result);
+                 }
+             }
+           break;
+         }
+       case PLUS_EXPR:
+       case MINUS_EXPR:
+       case MULT_EXPR:
+       case LSHIFT_EXPR:
+         {
+           tree type = TREE_TYPE (lhs);
+           type = signed_or_unsigned_type_for (! TYPE_UNSIGNED (type), 
type);
+           tree ops[3];
+           ops[0] = gimple_assign_rhs1 (stmt);
+           ops[0] = vn_nary_op_lookup_pieces (1, NOP_EXPR, type, ops, 
NULL);
+           if (ops[0])
+             {
+               ops[1] = gimple_assign_rhs2 (stmt);
+               if (code != LSHIFT_EXPR)
+                 ops[1] = vn_nary_op_lookup_pieces (1, NOP_EXPR, type,
+                                                    &ops[1], NULL);
+               if (ops[1])
+                 {
+                   ops[0] = vn_nary_op_lookup_pieces (2, code, type,
+                                                      ops, NULL);
+                   if (ops[0])
+                     {
+                       result = vn_nary_build_or_lookup (NOP_EXPR,
+                                                         TREE_TYPE (lhs), 
ops);
+                       if (result)
+                         return set_ssa_val_to (lhs, result);
+                     }
+                 }
+             }
+           break;
+         }
+       default:;
+       }
     }

+  bool changed = set_ssa_val_to (lhs, lhs);
+  vn_nary_op_insert_stmt (stmt, lhs);
   return changed;
 }

Reply via email to