This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug tree-optimization/70600] Missed tree optimization with multiple additions in different types.


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

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 11 Jul 2016, pinskia at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70600
> 
> Andrew Pinski <pinskia at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>            Keywords|                            |missed-optimization
>                  CC|                            |pinskia at gcc dot gnu.org
>            Severity|normal                      |enhancement
> 
> --- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> I have seen other cases which need to be handled by FRE/PRE instead of
> reassiocate.
> 
> Even the simple:
> 
> int f(int a, int b, int d)
> {
>   unsigned a1 = a;
>   unsigned b1 = b;
>   unsigned c1 = a + b;
>   int c = c1;
>   if (d)
>     c = a + b;
>   return c;    
> }
> 
> Note I think FRE handles the above incorrectly as it converts the safe unsigned
> addition to non safe signed addition. I will file a different bug about that
> issue.

It's going to be interesting to teach FRE this equivalence ;)  I assume
you wanted to write

>   unsigned c1 = a1 + b1;

even though it doesn't make a difference in the IL.

It may be interesting to try transitioning GIMPLE to singed/unsigned
ops rather than relying on types (so be more similar to RTL in this
regard).  That would of course fix this.

The way FRE works with having expression tables that only cover
a single GIMPLE stmt it would require looking up unsigned
variants of a and b and then looking up the sum of both.  If found
we'd need to possibly insert a conversion.  I think it's possible
to teach FRE to do this in a quite generic way for all ops that
are sign-agnostic.  But doing this even for truncated ops and
thus using a truncated widened op might is going to be expensive
and not always profitable.

Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c        (revision 238237)
+++ gcc/tree-ssa-sccvn.c        (working copy)
@@ -3384,17 +3430,54 @@ 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))
+    {
+      enum tree_code code = gimple_assign_rhs_code (stmt);
+      switch (code)
+       {
+       case PLUS_EXPR:
+       case MINUS_EXPR:
+       case MULT_EXPR:
+         {
+           tree ops[3];
+           ops[0] = gimple_assign_rhs1 (stmt);
+           ops[1] = gimple_assign_rhs2 (stmt);
+           tree type = TREE_TYPE (lhs);
+           type = signed_or_unsigned_type_for (! TYPE_UNSIGNED (type),
+                                                    type);
+           ops[0] = vn_nary_op_lookup_pieces (1, NOP_EXPR, type, ops, 
NULL);
+           if (ops[0])
+             {
+               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;
 }


produces in t.c.034t.fre1

f (int a, int b, int d)
{
  int c;
  unsigned int c1;
  unsigned int b1;
  unsigned int a1;
  _Bool _1;
  _Bool _2;
  _Bool _3;
  int _5;

  <bb 2>:
  a1_7 = (unsigned int) a_6(D);
  b1_9 = (unsigned int) b_8(D);
  c1_10 = a1_7 + b1_9;
  _1 = a_6(D) <= 0;
  _2 = b_8(D) <= 0;
  _3 = _1 | _2;
  if (_3 != 0)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 3>:
  c_12 = (int) c1_10;

  <bb 4>:
  # _5 = PHI <0(2), c_12(3)>
  return _5;

}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]