> 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" } } */
