The attached patch adds attributes nonnull and pure to
tree_to_shwi() and a small number of other heavily used functions
that will benefit from both.
First, I noticed that there are a bunch of functions in tree.c that
deal gracefully with null pointers right alongside a bunch of other
related functions that don't.
For example, tree_fits_shwi_p() is null-safe but integer_zerop()
is not. There a number of others. I never remember which ones
are in which group and so I either add unnecessary checks or
forget to add them, for which we then all pay when the missing
check triggers an ICE. In patch reviews I see I'm not the only
one who's not sure. The attribute should help avoid some of
these problems: both visually and via warnings.
Second, while testing the nonnull patch, I noticed that GCC would
not optimize some null tests after calls to nonnull functions that
take tree as an argument and that I expected it to optimize, like
n = tree_to_shwi (TYPE_SIZE (type));
if (TYPE_SIZE (type))
...
The reason is that tree_to_shwi() isn't declared pure, even though
tree_fits_shwi_p() is (though as I mentioned, the latter is null
safe and so not declarted nonnull, and so it doesn't make the same
optimization possible).
Tested on x86_64-linux. The patch exposed a couple of issues
in Ada. At least the first one is a false positive caused by
GCC being unaware that tree_fits_uhwi_p() returns false for
a null argument (propagating this knowledge via an attribute
seems like an opportunity for a future enhancement).
I suppressed the warning by introducing a local temporary.
I suspect the second warning is caused by the Ada TYPE_RM_SIZE()
macro expanding to a conditional with an explicit null operand:
#define TYPE_RM_SIZE(NODE) TYPE_RM_VALUE ((NODE), 0)
#define TYPE_RM_VALUE(NODE, N) \
(TYPE_RM_VALUES (NODE) \
? TREE_VEC_ELT (TYPE_RM_VALUES (NODE), (N)) : NULL_TREE)
but I wasn't able to reduce it to a small test case to confirm
that. I suppressed this warning by introducing a temporary as
well.
Martin
gcc/ChangeLog:
* tree.h (tree_to_shwi): Add attribute nonnull.
(tree_to_poly_int64, tree_to_uhwi, tree_to_poly_uint64): Same.
(integer_zerop, integer_onep, int_fits_type_p): Same.
gcc/ada/ChangeLog:
* gcc-interface/utils.c (make_packable_type): Introduce a temporary
to avoid -Wnonnull.
(unchecked_convert): Same.
Index: gcc/ada/gcc-interface/utils.c
===================================================================
--- gcc/ada/gcc-interface/utils.c (revision 264652)
+++ gcc/ada/gcc-interface/utils.c (working copy)
@@ -990,15 +990,16 @@ make_packable_type (tree type, bool in_record, uns
}
else
{
+ tree type_size = TYPE_ADA_SIZE (type);
/* Do not try to shrink the size if the RM size is not constant. */
if (TYPE_CONTAINS_TEMPLATE_P (type)
- || !tree_fits_uhwi_p (TYPE_ADA_SIZE (type)))
+ || !tree_fits_uhwi_p (type_size))
return type;
/* Round the RM size up to a unit boundary to get the minimal size
for a BLKmode record. Give up if it's already the size and we
don't need to lower the alignment. */
- new_size = tree_to_uhwi (TYPE_ADA_SIZE (type));
+ new_size = tree_to_uhwi (type_size);
new_size = (new_size + BITS_PER_UNIT - 1) & -BITS_PER_UNIT;
if (new_size == size && (max_align == 0 || align <= max_align))
return type;
@@ -5307,20 +5308,21 @@ unchecked_convert (tree type, tree expr, bool notr
to its size, sign- or zero-extend the result. But we need not do this
if the input is also an integral type and both are unsigned or both are
signed and have the same precision. */
+ tree type_rm_size;
if (!notrunc_p
&& INTEGRAL_TYPE_P (type)
&& !(code == INTEGER_TYPE && TYPE_BIASED_REPRESENTATION_P (type))
- && TYPE_RM_SIZE (type)
- && tree_int_cst_compare (TYPE_RM_SIZE (type), TYPE_SIZE (type)) < 0
+ && (type_rm_size = TYPE_RM_SIZE (type))
+ && tree_int_cst_compare (type_rm_size, TYPE_SIZE (type)) < 0
&& !(INTEGRAL_TYPE_P (etype)
&& type_unsigned_for_rm (type) == type_unsigned_for_rm (etype)
&& (type_unsigned_for_rm (type)
- || tree_int_cst_compare (TYPE_RM_SIZE (type),
+ || tree_int_cst_compare (type_rm_size,
TYPE_RM_SIZE (etype)
? TYPE_RM_SIZE (etype)
: TYPE_SIZE (etype)) == 0)))
{
- if (integer_zerop (TYPE_RM_SIZE (type)))
+ if (integer_zerop (type_rm_size))
expr = build_int_cst (type, 0);
else
{
@@ -5330,7 +5332,7 @@ unchecked_convert (tree type, tree expr, bool notr
tree shift_expr
= convert (base_type,
size_binop (MINUS_EXPR,
- TYPE_SIZE (type), TYPE_RM_SIZE (type)));
+ TYPE_SIZE (type), type_rm_size));
expr
= convert (type,
build_binary_op (RSHIFT_EXPR, base_type,
Index: gcc/tree.h
===================================================================
--- gcc/tree.h (revision 264653)
+++ gcc/tree.h (working copy)
@@ -4231,16 +4231,23 @@ extern tree purpose_member (const_tree, tree);
extern bool vec_member (const_tree, vec<tree, va_gc> *);
extern tree chain_index (int, tree);
+/* Arguments may be null. */
extern int tree_int_cst_equal (const_tree, const_tree);
+/* The following predicates are safe to call with a null argument. */
extern bool tree_fits_shwi_p (const_tree) ATTRIBUTE_PURE;
extern bool tree_fits_poly_int64_p (const_tree) ATTRIBUTE_PURE;
extern bool tree_fits_uhwi_p (const_tree) ATTRIBUTE_PURE;
extern bool tree_fits_poly_uint64_p (const_tree) ATTRIBUTE_PURE;
-extern HOST_WIDE_INT tree_to_shwi (const_tree);
-extern poly_int64 tree_to_poly_int64 (const_tree);
-extern unsigned HOST_WIDE_INT tree_to_uhwi (const_tree);
-extern poly_uint64 tree_to_poly_uint64 (const_tree);
+
+extern HOST_WIDE_INT tree_to_shwi (const_tree)
+ ATTRIBUTE_NONNULL (1) ATTRIBUTE_PURE;
+extern poly_int64 tree_to_poly_int64 (const_tree)
+ ATTRIBUTE_NONNULL (1) ATTRIBUTE_PURE;
+extern unsigned HOST_WIDE_INT tree_to_uhwi (const_tree)
+ ATTRIBUTE_NONNULL (1) ATTRIBUTE_PURE;
+extern poly_uint64 tree_to_poly_uint64 (const_tree)
+ ATTRIBUTE_NONNULL (1) ATTRIBUTE_PURE;
#if !defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 4003)
extern inline __attribute__ ((__gnu_inline__)) HOST_WIDE_INT
tree_to_shwi (const_tree t)
@@ -4893,7 +4900,8 @@ extern bool really_constant_p (const_tree);
extern bool ptrdiff_tree_p (const_tree, poly_int64_pod *);
extern bool decl_address_invariant_p (const_tree);
extern bool decl_address_ip_invariant_p (const_tree);
-extern bool int_fits_type_p (const_tree, const_tree);
+extern bool int_fits_type_p (const_tree, const_tree)
+ ATTRIBUTE_NONNULL (1) ATTRIBUTE_NONNULL (2) ATTRIBUTE_PURE;
#ifndef GENERATOR_FILE
extern void get_type_static_bounds (const_tree, mpz_t, mpz_t);
#endif
