On Fri, Sep 20, 2024 at 12:40:27PM -0400, Siddhesh Poyarekar wrote:
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -1468,6 +1468,63 @@ merge_object_sizes (struct object_size_info *osi, tree
> dest, tree orig)
> return bitmap_bit_p (osi->reexamine, SSA_NAME_VERSION (orig));
> }
>
> +/* For constant sizes, try collapsing a non-constant offset to a constant if
> + possible. The case handled at the moment is when the offset is a PHI node
> + with all of its targets are constants. */
> +
> +static tree
> +try_collapsing_offset (tree op, int object_size_type)
> +{
> + gcc_assert (!(object_size_type & OST_DYNAMIC));
> +
> + if (TREE_CODE (op) != SSA_NAME)
> + return op;
> +
> + gimple *stmt = SSA_NAME_DEF_STMT (op);
> +
> + switch (gimple_code (stmt))
> + {
> + case GIMPLE_ASSIGN:
> + /* Peek through casts. */
> + if (gimple_assign_rhs_code (stmt) == NOP_EXPR)
Do you really want to handle all casts? That could be widening, narrowing,
from non-integral types, ...
If only same mode, then gimple_assign_unary_nop_p probably should be used
instead, if any from integral types (same precision, widening, narrowing),
then perhaps CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt))
but verify that INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt)))
before actually recursing.
Note, I think narrowing or widening casts to sizetype are fine, but when
you recurse through, other casts might not be anymore.
Consider
short int _1;
unsigned int _2;
size_t _3;
...
# _1 = PHI <-10(7), 12(8), 4(9), -42(10)>
_2 = (unsigned int) _1;
_3 = (size_t) _2;
If the recursion finds minimum or maximum from the signed short int _1
values (cast to ptrdiff_type_node), pretending that is the minimum or
maximum for _3 is just wrong, as the cast from signed to unsigned will
turn negative values to something larger than the smaller positive values.
Similarly, consider casts from unsigned __int128 -> unsigned short -> size_t
(or signed short in between), what is minimum/maximum in 128-bits (the code
for PHIs actually ignores the upper bits and looks only for signed 64-bits,
but if there is unfolded cast from INTEGER_CST you actually could have even
large value) isn't necessarily minimum after cast to 16-bit (unsigned or
signed).
So, unless you want to get all the possibilities into account, perhaps only
recurse through casts from integer types to integer types with precision
of sizetype?
And perhaps also look for INTEGER_CST type returned from the recursive
call and if it doesn't have sizetype precision, either convert it to
sizetype or ignore.
> + {
> + tree ret = try_collapsing_offset (gimple_assign_rhs1 (stmt),
> + object_size_type);
> + if (TREE_CODE (ret) == INTEGER_CST)
> + return ret;
> + }
> + break;
> + case GIMPLE_PHI:
> + {
> + tree off = ((object_size_type & OST_MINIMUM)
> + ? TYPE_MIN_VALUE (ptrdiff_type_node)
> + : TYPE_MAX_VALUE (ptrdiff_type_node));
Because you only process constants, I wonder whether using
wide_int off and performing the min/max on wide_int wouldn't be
better, no need to create INTEGER_CSTs for all the intermediate
results.
That would be
unsigned int prec = TYPE_PRECISION (ptrdiff_type_node);
wide_int off = ((object_size_type & OST_MINIMUM)
? wi::to_wide (TYPE_MIN_VALUE (ptrdiff_type_node))
: wi::to_wide (TYPE_MAX_VALUE (ptrdiff_type_node)));
> +
> + for (unsigned i = 0; i < gimple_phi_num_args (stmt); i++)
> + {
> + tree rhs = gimple_phi_arg_def (stmt, i);
> +
I wonder if it wouldn't be useful to recurse here,
rhs = try_collapsing_offset (rhs, object_size_type);
but guess with some extra counter argument and only allow some small
constant levels of nesting (but also do that for the cast cases).
> + if (TREE_CODE (rhs) != INTEGER_CST)
> + return op;
> +
> + /* Note that this is the *opposite* of what we usually do with
> + sizes, because the maximum offset estimate here will give us a
> + minimum size estimate and vice versa. */
> + enum tree_code code = (object_size_type & OST_MINIMUM
> + ? MAX_EXPR : MIN_EXPR);
> +
> + off = fold_build2 (code, ptrdiff_type_node, off,
> + fold_convert (ptrdiff_type_node, rhs));
And this could be
wide_int w = wi::to_wide (rhs, prec);
if (object_size_type & OST_MINIMUM)
off = wi::smax (off, w);
else
off = wi::smin (off, w);
> + }
> + return fold_convert (sizetype, off);
return wide_int_to_tree (sizetype, off);
> + }
> + default:
> + break;
> + }
> +
> + /* Nothing worked, so return OP untouched. */
> + return op;
> +}
>
> /* Compute object_sizes for VAR, defined to the result of an assignment
> with operator POINTER_PLUS_EXPR. Return true if the object size might
> @@ -1500,6 +1557,9 @@ plus_stmt_object_size (struct object_size_info *osi,
> tree var, gimple *stmt)
> if (object_sizes_unknown_p (object_size_type, varno))
> return false;
>
> + if (!(object_size_type & OST_DYNAMIC) && TREE_CODE (op1) != INTEGER_CST)
> + op1 = try_collapsing_offset (op1, object_size_type);
> +
> /* Handle PTR + OFFSET here. */
> if (size_valid_p (op1, object_size_type)
> && (TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
> --
> 2.45.1
Jakub
