This is the mail archive of the gcc-patches@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]

Re: [RFC] optimize x - y cmp 0 with undefined overflow


> Looks mostly ok.  Any reason why you are not re-creating
> MINUS_EXPR in build_symbolic_expr?  That is, build
> inv - t (for non-pointers, of course)?

It's more uniform and compare_values expects an INTEGER_CST on the RHS, 
although the patch was lacking a small tweak to the function:

@@ -1205,19 +1292,23 @@ compare_values_warnv (tree val1, tree va
   STRIP_USELESS_TYPE_CONVERSION (val2);
 
   if ((TREE_CODE (val1) == SSA_NAME
+       || (TREE_CODE (val1) == NEGATE_EXPR
+          && TREE_CODE (TREE_OPERAND (val1, 0)) == SSA_NAME)
        || TREE_CODE (val1) == PLUS_EXPR
        || TREE_CODE (val1) == MINUS_EXPR)
       && (TREE_CODE (val2) == SSA_NAME
+         || (TREE_CODE (val2) == NEGATE_EXPR
+             && TREE_CODE (TREE_OPERAND (val2, 0)) == SSA_NAME)
          || TREE_CODE (val2) == PLUS_EXPR
          || TREE_CODE (val2) == MINUS_EXPR))
     {
       tree n1, c1, n2, c2;
       enum tree_code code1, code2;
 
-      /* If VAL1 and VAL2 are of the form 'NAME [+-] CST' or 'NAME',
+      /* If VAL1 and VAL2 are of the form '[-]NAME [+-] CST' or 'NAME',
         return -1 or +1 accordingly.  If VAL1 and VAL2 don't use the
         same name, return -2.  */
-      if (TREE_CODE (val1) == SSA_NAME)
+      if (TREE_CODE (val1) == SSA_NAME || TREE_CODE (val1) == NEGATE_EXPR)
        {
          code1 = SSA_NAME;
          n1 = val1;
@@ -1239,7 +1330,7 @@ compare_values_warnv (tree val1, tree va
            }
        }
 
-      if (TREE_CODE (val2) == SSA_NAME)
+      if (TREE_CODE (val2) == SSA_NAME || TREE_CODE (val2) == NEGATE_EXPR)
        {
          code2 = SSA_NAME;
          n2 = val2;
@@ -1262,6 +1353,11 @@ compare_values_warnv (tree val1, tree va
        }
 
       /* Both values must use the same name.  */
+      if (TREE_CODE (n1) == NEGATE_EXPR && TREE_CODE (n2) == NEGATE_EXPR)
+       {
+         n1 = TREE_OPERAND (n1, 0);
+         n2 = TREE_OPERAND (n2, 0);
+       }
       if (n1 != n2)
        return -2;
 
> Otherwise if a range becomes -t + inv that will no longer match
> get_single_symbol for further propagation?

Yes, it will, NEGATE_EXPR is handled by get_single_symbol.

> Then I'm not sure if
> 
> +      /* Try with VR0 and [-INF, OP1].  */
> +      set_value_range (&new_vr1, VR_RANGE, vrp_val_min (expr_type), op1,
> NULL); +      extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
> &new_vr1); +      if (vr->type != VR_VARYING)
> +       return;
> +
> +      /* Try with VR0 and [OP1, +INF].  */
> +      set_value_range (&new_vr1, VR_RANGE, op1, vrp_val_max (expr_type),
> NULL); +      extract_range_from_binary_expr_1 (vr, code, expr_type, &vr0,
> &new_vr1); +      if (vr->type != VR_VARYING)
> +       return;
> 
> is a safe thing to do.  If it does make a difference to try [-INF, OP1],
> [OP1, +INF] instead of just [OP1, OP1] then at least it's very suspicious ;)
> (or an "easy" missed optimization).

Yes, it makes a difference and is required to eliminate half-symbolic ranges 
(the ones deduced from x >= y).  Otherwise extract_range_from_binary_expr_1 
will reject the resulting range because it cannot compare the bounds.

As discussed, extract_range_from_binary_expr_1 doesn't do any fiddling with 
the input ranges, it just computes the resulting range and see whether it can 
interpret it.  Half-symbolic ranges cannot be interpreted and probably should 
not, in the general case at least, because I think it can be very delicate, so 
extract_range_from_binary_expr will just try all the possible combinations for 
PLUS and MINUS.

The testcases were attached to the first message in the thread, but I presume 
that decent mail programs are a thing of the past. :-)  Attached again.

-- 
Eric Botcazou
-- { dg-do compile }
-- { dg-options "-O2 -fdump-tree-optimized" }

function Opt38 (X : Integer; Y : Integer ) return Positive is
   Z : Integer;
begin
   if X >= Y then
      return 1;
   end if;
   Z := Y - X;
   return Z;
end;

-- { dg-final { scan-tree-dump-not "gnat_rcheck" "optimized" } }
-- { dg-final { cleanup-tree-dump "optimized" } }
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

extern void abort (void);

int foo1 (int x, int y)
{
  int z;

  if (x >= y)
    return 1;

  z = y - x;
  if (z <= 0)
    abort ();

  return z;
}

unsigned int foo2 (unsigned int x, unsigned int y)
{
  unsigned int z;

  if (x >= y)
    return 1;

  z = y - x;
  if (z == 0)
    abort ();

  return z;
}

/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */

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