[gcc(refs/vendors/ARM/heads/morello)] Avoid incompatible conversion in fold_build_replace_address_value_loc
Matthew Malcomson
matmal01@gcc.gnu.org
Fri Oct 1 09:17:09 GMT 2021
https://gcc.gnu.org/g:7f0d8feb2bddbc4300577c8400b32fe4f4760376
commit 7f0d8feb2bddbc4300577c8400b32fe4f4760376
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date: Tue Sep 21 17:47:56 2021 +0100
Avoid incompatible conversion in fold_build_replace_address_value_loc
The idea here was to change something of the form
ifn_REPLACE_ADDRESS_VALUE (<nop <void *> <char * 10>>, 100)
into
<nop <void *> <ifn_REPLACE_ADDRESS_VALUE (<char * 10>, 100)>
allowing the folding routines to recognise
ifn_REPLACE_ADDRESS_VALUE (<char * 10>, 100)
and turn it into
<char * 100>
Unfortunately, we allowed such transformations on any types -- ignoring
the type conversion. This meant that we could end up with an
`ifn_REPLACE_ADDRESS_VALUE` call whose two arguments did not match in
types (i.e. noncapability_type (arg1) was not `types_compatible_p` with
arg0).
Here we add an extra check to avoid performing the optimisation in such
cases. This bumps into an assertion in `initializer_constant_valid_p_1`
that we only ever see a REPLACE_ADDRESS_VALUE call as an initialiser
when the capability we are using is a constant with zero metadata.
That assertion was largely about ensuring we know what cases we may see
rather than for correctness. The main point is to know that we never
initialise capabilities with metadata. Here we update the assertion to
allow some number of NOP_EXPR, and REPLACE_ADDRESS_VALUE casts from a
constant capability with cleared metadata.
That means we can still double check there is no constant value setting
metadata on any capability.
Diff:
---
gcc/fold-const.c | 11 +++++--
.../morello/invalid-replace-address-value.c | 11 +++++++
gcc/varasm.c | 38 +++++++++++++++++++---
3 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 08d54b98d05..ec11bf463d8 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13583,8 +13583,11 @@ fold_build_replace_address_value_loc (location_t loc, tree c, tree cv)
}
if (TREE_CODE (c) == CALL_EXPR && CALL_EXPR_FN (c) == NULL_TREE
&& CALL_EXPR_IFN (c) == IFN_REPLACE_ADDRESS_VALUE)
- return fold_build_replace_address_value_loc (loc,
- CALL_EXPR_ARG (c, 0), cv);
+ {
+ gcc_assert (TREE_TYPE (c) == TREE_TYPE (CALL_EXPR_ARG (c, 0)));
+ return fold_build_replace_address_value_loc (loc,
+ CALL_EXPR_ARG (c, 0), cv);
+ }
/* If the capability C is an INTEGER_CST or a REPLACE_ADDRESS_VALUE inside
a NOP conversion, allow this function to recurse and re-apply the
conversion on the result. */
@@ -13592,7 +13595,9 @@ fold_build_replace_address_value_loc (location_t loc, tree c, tree cv)
&& ((TREE_CODE (TREE_OPERAND (c, 0)) == CALL_EXPR
&& CALL_EXPR_IFN (TREE_OPERAND (c, 0)) == IFN_REPLACE_ADDRESS_VALUE
&& CALL_EXPR_FN (TREE_OPERAND (c, 0)) == NULL_TREE)
- || TREE_CODE (TREE_OPERAND (c, 0)) == INTEGER_CST))
+ || TREE_CODE (TREE_OPERAND (c, 0)) == INTEGER_CST)
+ && types_compatible_p (noncapability_type (TREE_TYPE (TREE_OPERAND (c, 0))),
+ TREE_TYPE (cv)))
return convert (TREE_TYPE (c), fold_build_replace_address_value_loc (loc,
TREE_OPERAND (c, 0), cv));
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/invalid-replace-address-value.c b/gcc/testsuite/gcc.target/aarch64/morello/invalid-replace-address-value.c
new file mode 100644
index 00000000000..12cc9f03fcf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/invalid-replace-address-value.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* This would previously trigger an ICE in expand_REPLACE_ADDRESS_VALUE due to
+ an invalid optimisation. */
+typedef __intcap_t a;
+typedef long unsigned b;
+b c, d;
+int e;
+int f() {
+ d = d - (a)c;
+ return e;
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index c3c0d1959a5..b0b1e7c2c93 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -4548,6 +4548,32 @@ narrowing_initializer_constant_valid_p (tree value, tree endtype, tree *cache)
return NULL_TREE;
}
+/* Helper function for initializer_constant_valid_p_1.
+ This iterates down all the kinds of conversion that we can expect to see in
+ a global initializer for a capability. The idea is to help check that on
+ all global capabilities with initializers we start at some point with a
+ capabiility that has zero metadata (and hence that all initializers have
+ cleared metadata).
+
+ That should ensure we don't start handling capabilities with non-zero
+ metadata without knowing why & where it comes from. */
+inline const_tree
+tree_innermost_capability (const_tree t)
+{
+ switch (TREE_CODE (t))
+ {
+ case INTEGER_CST:
+ return t;
+ case NOP_EXPR:
+ return tree_innermost_capability (TREE_OPERAND (t, 0));
+ case CALL_EXPR:
+ gcc_assert (CALL_EXPR_IFN (t) == IFN_REPLACE_ADDRESS_VALUE);
+ return tree_innermost_capability (CALL_EXPR_ARG (t, 0));
+ default:
+ gcc_unreachable ();
+ }
+}
+
/* Helper function of initializer_constant_valid_p.
Return nonzero if VALUE is a valid constant-valued expression
for use in initializing a static variable; one that can be an
@@ -4580,11 +4606,13 @@ initializer_constant_valid_p_1 (tree value, tree endtype, tree *cache)
|| CALL_EXPR_IFN (ptr) != IFN_REPLACE_ADDRESS_VALUE);
if (TREE_CODE (ptr) == CALL_EXPR)
break;
- /* MORELLO TODO Just adding this assert in temporarily. I would be
- interested to see any time that this is not the case and hence want
- to get alerted. */
- gcc_assert (TREE_CODE (ptr) == INTEGER_CST
- && tree_constant_capability_metadata (ptr) == 0);
+ /* MORELLO TODO Here we assert that we only ever see capability
+ initialisers where the metadata is known to be cleared.
+ Any time this is not the case is likely to be a bug (at least until
+ we add some feature to do such things). */
+ const_tree base_cap = tree_innermost_capability (ptr);
+ gcc_assert ((TREE_CODE (base_cap) == INTEGER_CST
+ && tree_constant_capability_metadata (base_cap) == 0));
tree ptr_ret = initializer_constant_valid_p_1 (ptr, endtype, cache);
tree addrval_ret = initializer_constant_valid_p_1
(addr_value, noncapability_type (endtype), cache);
More information about the Gcc-cvs
mailing list