public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] morello: Improve handling of REPLACE_ADDRESS_VALUE constants
@ 2022-04-06  9:59 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2022-04-06  9:59 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:9c9a99d71e59ec0024edbba90ba331ff0be9f409

commit 9c9a99d71e59ec0024edbba90ba331ff0be9f409
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon Mar 7 15:42:04 2022 +0000

    morello: Improve handling of REPLACE_ADDRESS_VALUE constants
    
    The initial problem here was an ICE in
    varasm.c:tree_innermost_capability via
    varasm.c:initializer_constant_valid_p_1. The code in these functions
    only expected to see an INTEGER_CST as the base capability in a
    REPLACE_ADDRESS_VALUE. However, we can of course derive an __intcap from
    a pointer, simply by casting the pointer to an __intcap (and optionally
    adding an offset), as in the testcase added with this patch.
    
    We adapt the code in the above functions to handle this case.
    
    After making this change, it was found that the backend couldn't handle
    assembling the resulting rtxes on pure-capability Morello. This can be
    remedied by introducing a new RTL simplification rule to transform:
    
    (replace_address_value:M C (plus:OM C.CV X)) --> (pointer_plus:M C X).
    
    where M is a capability mode, OM is its offset mode, C is a capability, C.CV is
    its address value, and X is an arbitrary rtx.
    
    The backend can of course already handle the POINTER_PLUS form.
    
    Note that this bug was originally triggered by a C++ testcase,
    g++.dg/cpp0x/pr88410.C. However, the underlying issue can also be triggered from
    C, as with the testcase in this patch.
    
    gcc/ChangeLog:
    
            * simplify-rtx.c (lowparts_p): If the simplified rtxes are not
            registers, return true if they compare equal by pointer
            equality. This handles e.g. subregs of SYMBOL_REFs.
            (simplify_binary_operation_1): New rule to simplify
            REPLACE_ADDRESS_VALUE to POINTER_PLUS where possible.
            (test_replace_address_value_simplifications): New test.
            * varasm.c (tree_innermost_capability): Also handle ADDR_EXPR as
            the innermost capability.
            (initializer_constant_valid_p_1): Handle constants derived from
            ADDR_EXPRs.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.target/aarch64/morello/intcap-pointer-plus-ice.c: New test.

Diff:
---
 gcc/simplify-rtx.c                                 | 27 +++++++++++++++++-
 .../aarch64/morello/intcap-pointer-plus-ice.c      |  6 ++++
 gcc/varasm.c                                       | 33 ++++++++++++++++------
 3 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 36831d5566a..b209a9a419b 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -82,7 +82,7 @@ lowparts_p (rtx x, machine_mode mx, rtx y, machine_mode my)
     return CONST_INT_P (right) && INTVAL (left) == INTVAL (right);
 
   if (!REG_P (left) || !REG_P (right))
-    return false;
+    return left == right;
   else if (!HARD_REGISTER_P (left) || !HARD_REGISTER_P (right))
     return REGNO (left) == REGNO (right);
   else
@@ -4469,6 +4469,14 @@ simplify_binary_operation_1 (enum rtx_code code, machine_mode mode,
 					mode, op0, GEN_INT (align_const));
 	}
 
+      if (GET_CODE (op1) == PLUS
+	  && lowparts_p (op0, mode, XEXP (op1, 0), om))
+	{
+	  return simplify_gen_binary (POINTER_PLUS,
+				      mode,
+				      op0, XEXP (op1, 1));
+	}
+
       return 0;
     }
 
@@ -8314,6 +8322,23 @@ test_replace_address_value_simplifications ()
 
       /* TODO: test case with side effects?  */
     }
+
+  /* (replace_address_value:M C (plus:OM C.CV X))
+     --> (pointer_plus:M C X)
+
+     Test it with SYMBOL_REFs here, as that is our motivating case.  */
+  rtx sym = gen_rtx_SYMBOL_REF (cm, "x");
+  x = simplify_gen_binary (REPLACE_ADDRESS_VALUE,
+			   cm,
+			   sym,
+			   gen_rtx_PLUS (om,
+					 drop_capability (sym),
+					 constm1_rtx));
+  if (GET_CODE (x) == CONST)
+    x = XEXP (x, 0);
+  ASSERT_EQ (GET_CODE (x), POINTER_PLUS);
+  ASSERT_EQ (XEXP (x, 0), sym);
+  ASSERT_EQ (XEXP (x, 1), constm1_rtx);
 }
 
 static void
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/intcap-pointer-plus-ice.c b/gcc/testsuite/gcc.target/aarch64/morello/intcap-pointer-plus-ice.c
new file mode 100644
index 00000000000..8efd5a4ace5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/intcap-pointer-plus-ice.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* We used to ICE here in initializer_constant_valid_p_1 because we weren't
+   expecting to see anything other than an INTEGER_CST in a
+   REPLACE_ADDRESS_VALUE.  */
+int x;
+__intcap c = (__intcap)&x + 1;
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 48a880d4f66..2e6b98e6ef9 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -4626,6 +4626,7 @@ tree_innermost_capability (const_tree t)
   switch (TREE_CODE (t))
     {
     case INTEGER_CST:
+    case ADDR_EXPR:
       return t;
     case NOP_EXPR:
       return tree_innermost_capability (TREE_OPERAND (t, 0));
@@ -4669,18 +4670,34 @@ 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 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);
-	/* Return value doesn't matter beyond what the comment above mentions.
-	 * */
+
+	/* Handle cases like:
+	   (__intcap)&x + INTEGER_CST
+	   with IR like:
+	   .REPLACE_ADDRESS_VALUE (&x, (long)&x + INTEGER_CST)
+	   which later get expanded to POINTER_PLUS.  */
+	if (TREE_CODE (base_cap) == ADDR_EXPR)
+	  {
+	    tree av_addr = addr_value;
+	    if (TREE_CODE (av_addr) == PLUS_EXPR)
+	      av_addr = TREE_OPERAND (av_addr, 0);
+
+	    if (TREE_CODE (av_addr) == NOP_EXPR)
+	      av_addr = TREE_OPERAND (av_addr, 0);
+
+	    if (av_addr != base_cap)
+	      return 0;
+	  }
+	else
+	  gcc_assert ((TREE_CODE (base_cap) == INTEGER_CST
+		       && tree_constant_capability_metadata (base_cap) == 0));
+
 	if (ptr_ret == null_pointer_node && addrval_ret == null_pointer_node)
 	  return null_pointer_node;
 	if (ptr_ret && addrval_ret)


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-04-06  9:59 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-06  9:59 [gcc(refs/vendors/ARM/heads/morello)] morello: Improve handling of REPLACE_ADDRESS_VALUE constants Matthew Malcomson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).