[gcc(refs/vendors/ARM/heads/morello)] middle-end: Fix bogus folding of REPLACE_ADDRESS_VALUE

[gcc(refs/vendors/ARM/heads/morello)] middle-end: Fix bogus folding of REPLACE_ADDRESS_VALUE

[Alex Coplan]

https://gcc.gnu.org/g:e0ffb332e6fccabe8acb1806511262ae43da2ca3

commit e0ffb332e6fccabe8acb1806511262ae43da2ca3
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Thu May 26 10:59:24 2022 +0100

    middle-end: Fix bogus folding of REPLACE_ADDRESS_VALUE
    
    fold_build_replace_address_value_loc did the following transformation:
    
    RAV (RAV (c, cv1), cv2) -> cv2
    
    which is incorrect in the case where cv1 has side effects. It also
    (incorrectly) attempted to handle a similar case where the inner RAV was
    inside a NOP_EXPR.
    
    In the general case, both of these folds really seem to be optimizations
    that should be handled by the gimple optimizers rather than
    transformations that are necessary for constant folding in the FE.
    
    The folds we do in fold_build_replace_address_value_loc should really be
    the minimum necessary for constant folding in the FE. Any further
    optimization can be done on GIMPLE using match.pd rules.
    
    We adjust the nested RAV fold in fold_build_replace_address_value_loc to
    only handle the case where the inner RAV and the new capability value
    are both TREE_CONSTANT.
    
    This should be sufficient for FE constant folding (e.g. for the new test
    added with the patch), but without implementing any further
    optimization. This should also ensure we fix the issue where cv1 has
    side effects, since in this case the inner RAV would not be marked
    TREE_CONSTANT.
    
    This revealed that we weren't marking IFN CALL_EXPR nodes as
    TREE_CONSTANT even if both operands were TREE_CONSTANT, so we fix this
    by adjusting tree.c:process_call_operands.  Here we mark IFN calls as
    TREE_CONSTANT if they are flagged ICF_CONST and ICF_NOTHROW, and all of
    their operands are also TREE_CONSTANT. This further revealed that we
    hadn't given REPLACE_ADDRESS_VALUE IFNs the ECF_NOTHROW flag, so we add
    this flag in internal-fn.def.
    
    We add a match.pd rule to pick up the general optimization of nested
    REPLACE_ADDRESS_VALUE calls at the GIMPLE level. This prevents
    regressing now that we have restricted the generic folding to do the
    bare minimum for constant folding.
    
    We remove an overzealous assert from
    varasm.c:initializer_constant_valid_p_1. Here we attempted to enforce
    the invariant that we never see nested REPLACE_ADDRESS_VALUE calls, but
    it simply isn't possible to maintain this invariant in the general case,
    since these can't always be folded, so we simply reject these instead.
    
    gcc/ChangeLog:
    
            * fold-const.c (fold_build_replace_address_value_loc): Only do
            bare minimum needed for constant folding, thereby avoiding a
            correctness problem where we lose side effects.
            * internal-fn.def (REPLACE_ADDRESS_VALUE): Add ECF_NOTHROW.
            * match.pd (RAV (RAV (c, cv1), cv2) -> RAV(c, cv2)): New.
            * tree.c (process_call_operands): Propagate TREE_CONSTANT for
            IFNs marked ECF_CONST and ECF_NOTHROW.
            * varasm.c (initializer_constant_valid_p_1): Don't assert on
            nested REPLACE_ADDRESS_VALUE.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.target/aarch64/morello/intcap-global-const.c: New test.

Diff:
---
 gcc/fold-const.c                                         | 16 +++-------------
 gcc/internal-fn.def                                      |  2 +-
 gcc/match.pd                                             |  6 ++++++
 .../gcc.target/aarch64/morello/intcap-global-const.c     |  3 +++
 gcc/tree.c                                               | 11 +++++++++++
 gcc/varasm.c                                             |  3 ---
 6 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 07c54a87bba..384d3c149eb 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13603,25 +13603,15 @@ fold_build_replace_address_value_loc (location_t loc, tree c, tree cv)
 			     !POINTER_TYPE_P (cap_type),
 			     TREE_OVERFLOW (cv));
     }
+
   if (TREE_CODE (c) == CALL_EXPR && CALL_EXPR_FN (c) == NULL_TREE
-      && CALL_EXPR_IFN (c) == IFN_REPLACE_ADDRESS_VALUE)
+      && CALL_EXPR_IFN (c) == IFN_REPLACE_ADDRESS_VALUE
+      && TREE_CONSTANT (c) && TREE_CONSTANT (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), orig_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.  */
-  if (TREE_CODE (c) == NOP_EXPR
-      && ((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)
-      && 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), orig_cv));
 
   return build_replace_address_value_loc (loc, orig_c, orig_cv);
 }
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index f27c0a16750..a7a280894fb 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -394,7 +394,7 @@ DEF_INTERNAL_FN (CO_FRAME, ECF_PURE | ECF_NOTHROW | ECF_LEAF, NULL)
    actually have to be properly passed to the function (which is *not* valid)
    -- but it would be worth checking if it only means that the argument is not
    dereferenced.  */
-DEF_INTERNAL_FN (REPLACE_ADDRESS_VALUE, ECF_CONST | ECF_LEAF, ".RR")
+DEF_INTERNAL_FN (REPLACE_ADDRESS_VALUE, ECF_CONST | ECF_LEAF | ECF_NOTHROW, ".RR")
 
 DEF_INTERNAL_FN (CAP_GLOBAL_DATA_GET, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 
diff --git a/gcc/match.pd b/gcc/match.pd
index 34eb9d2a92e..d959763ef8c 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -6582,4 +6582,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && TYPE_PRECISION (type) <= TYPE_PRECISION (TREE_TYPE (@1)))
    (convert @1)))
 
+/* RAV (RAV (c, cv1), cv2) -> RAV (c, cv2).  */
+(simplify
+  (IFN_REPLACE_ADDRESS_VALUE
+    (IFN_REPLACE_ADDRESS_VALUE @0 @1) @2)
+  (IFN_REPLACE_ADDRESS_VALUE @0 @2))
+
 #endif
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/intcap-global-const.c b/gcc/testsuite/gcc.target/aarch64/morello/intcap-global-const.c
new file mode 100644
index 00000000000..ce1d17d67ef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/intcap-global-const.c
@@ -0,0 +1,3 @@
+/* { dg-do compile } */
+int x;
+__intcap c = (__intcap)&x + 1 + 1;
diff --git a/gcc/tree.c b/gcc/tree.c
index dcd85eedb41..75b177cbb99 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -3806,6 +3806,7 @@ process_call_operands (tree t)
 {
   bool side_effects = TREE_SIDE_EFFECTS (t);
   bool read_only = false;
+  bool constant = false;
   int i = call_expr_flags (t);
 
   /* Calls have side-effects, except those to const or pure functions.  */
@@ -3815,6 +3816,10 @@ process_call_operands (tree t)
   if (i & ECF_CONST)
     read_only = true;
 
+  /* Only internal function calls can be TREE_CONSTANT.  */
+  if (!CALL_EXPR_FN (t) && (i & ECF_CONST) && (i & ECF_NOTHROW))
+    constant = true;
+
   if (!side_effects || read_only)
     for (i = 1; i < TREE_OPERAND_LENGTH (t); i++)
       {
@@ -3823,10 +3828,16 @@ process_call_operands (tree t)
 	  side_effects = true;
 	if (op && !TREE_READONLY (op) && !CONSTANT_CLASS_P (op))
 	  read_only = false;
+	if (op && !TREE_CONSTANT (op))
+	  constant = false;
       }
 
+  /* constant => !side_effects.  */
+  gcc_assert (!constant || !side_effects);
+
   TREE_SIDE_EFFECTS (t) = side_effects;
   TREE_READONLY (t) = read_only;
+  TREE_CONSTANT (t) = constant;
 }
 \f
 /* Return true if EXP contains a PLACEHOLDER_EXPR, i.e. if it represents a
diff --git a/gcc/varasm.c b/gcc/varasm.c
index a35048676d2..2e76f0f725b 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -4665,9 +4665,6 @@ initializer_constant_valid_p_1 (tree value, tree endtype, tree *cache)
 	  break;
 	tree ptr = CALL_EXPR_ARG (value, 0);
 	tree addr_value = CALL_EXPR_ARG (value, 1);
-	/* Assume VALUE has been folded as much as possible.  */
-	gcc_assert (TREE_CODE (ptr) != CALL_EXPR
-		    || CALL_EXPR_IFN (ptr) != IFN_REPLACE_ADDRESS_VALUE);
 	if (TREE_CODE (ptr) == CALL_EXPR)
 	  break;