public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] aarch64: Tweak validity of label/symbol_ref addresses
@ 2022-05-05 12:09 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2022-05-05 12:09 UTC (permalink / raw)
  To: gcc-cvs

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

commit c675754a1788ddd741fb310d8d7ee612b95264f5
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Wed Apr 13 09:58:41 2022 +0100

    aarch64: Tweak validity of label/symbol_ref addresses
    
    arch64_classify_address had code to allow:
    
      (mem (label_ref X))    [case (1)]
    
    for any access that is word-sized or greater.  It also had code
    to allow:
    
      (mem (symbol_ref X))   [case (2)]
    
    when -mpc-relative-literal-loads is in effect and X is the address
    of a function literal pool entry.  The assumption in both cases is
    that X is within the range of a PC-relative LDR instruction.
    
    The decision about whether:
    
      (set (reg R) (label/symbol_ref X))
    
    is valid is instead made by aarch64_classify_symbol, with
    SYMBOL_TINY_ABSOLUTE (despite the “ABSOLUTE”) meaning that
    X is within the range of a PC-relative ADR instruction.
    
    The two decisions were inconsistent in that, for the normal
    “small” code model, aarch64_classify_symbol classified both a
    normal label_ref and a normal symbol_ref as SYMBOL_SMALL_ABSOLUTE
    rather than SYMBOL_TINY_ABSOLUTE.  Moves therefore required a
    hi/lo split in cases where MEMs allowed a direct reference.
    
    This then raises the question of which version is correct.
    I think the move/aarch64_classify_symbol choice is right for
    label_refs and the mem/aarch64_classify_address choice is right
    for symbol_refs.
    
    This difference probably doesn't matter much in practice, since
    (a) we should rarely (if ever) directly load from a label_ref and
    (b) we should rarely (if ever) need to take the address of a constant
    pool entry.  But it helps later patches if we “fix” the inconsistency
    and then make arch64_classify_address use aarch64_classify_symbol
    rather than doing its own thing.
    
    Although the patch is/was supposed to have little effect on code
    generation, disallowing (1) has the effect of making label_refs
    no longer satisfy memory_address_p when they need to be split into
    hi/lo accesses.  This means that they are no longer satisfy
    CONSTANT_ADDRESS_P.
    
    The new behaviour makes label_refs consistent with symbol_refs:
    they satisfy CONSTANT_ADDRESS_P if they are within the range of
    direct PC-relative accesses, but don't satisfy it otherwise.
    However, the generic handling of the %c operand format is:
    
                else if (letter == 'c')
                  {
                    if (CONSTANT_ADDRESS_P (operands[opnum]))
                      output_addr_const (asm_out_file, operands[opnum]);
                    else
                      output_operand (operands[opnum], 'c');
                  }
    
    and so the aarch64 handling of %c now sees label_refs when
    it didn't previously.

Diff:
---
 gcc/config/aarch64/aarch64.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index a0b7febdb42..b503e5dfd65 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10059,16 +10059,9 @@ aarch64_classify_address (struct aarch64_address_info *info,
 
       if (ldp_stp_mode == VOIDmode
 	  && GET_MODE_SIZE (mode).is_constant (&const_size)
-	  && const_size >= 4)
-	{
-	  rtx sym, addend;
-
-	  split_const (x, &sym, &addend);
-	  return (GET_CODE (sym) == LABEL_REF
-		   || (GET_CODE (sym) == SYMBOL_REF
-		       && aarch64_pcrelative_literal_loads
-		       && aarch64_function_literal_pool_address_p (x)));
-	}
+	  && const_size >= 4
+	  && aarch64_classify_symbolic_expression (x) == SYMBOL_TINY_ABSOLUTE)
+	return true;
       return false;
 
     case LO_SUM:
@@ -10910,12 +10903,14 @@ aarch64_print_operand (FILE *f, rtx x, int code)
 	  break;
 
 	case SYMBOL_REF:
+	case LABEL_REF:
 	  output_addr_const (f, x);
 	  break;
 
 	case CONST:
 	  if (any_plus_p (XEXP (x, 0))
-	      && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF)
+	      && (GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF
+		  || GET_CODE (XEXP (XEXP (x, 0), 0)) == LABEL_REF))
 	    {
 	      output_addr_const (f, x);
 	      break;
@@ -17115,6 +17110,13 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
       if (aarch64_sym_indirectly_accessed_p (x))
 	return aarch64_classify_capability_symbol (x, offset);
 
+      /* -mpc-relative-literal-loads tells us to assume that all (function)
+	 constant pool entries will be within the range of PC-relative LDR,
+	 which means that they must also be in range of ADR.  */
+      if (aarch64_pcrelative_literal_loads
+	  && aarch64_function_literal_pool_address_p (x))
+	return SYMBOL_TINY_ABSOLUTE;
+
       switch (aarch64_cmodel)
 	{
 	case AARCH64_CMODEL_TINY:
@@ -17165,8 +17167,7 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
 	  /* This is alright even in PIC code as the constant
 	     pool reference is always PC relative and within
 	     the same translation unit.  */
-	  if (!aarch64_pcrelative_literal_loads
-	      && aarch64_function_literal_pool_address_p (x))
+	  if (aarch64_function_literal_pool_address_p (x))
 	    return SYMBOL_SMALL_ABSOLUTE;
 	  else
 	    return SYMBOL_FORCE_TO_MEM;


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

only message in thread, other threads:[~2022-05-05 12:09 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 12:09 [gcc(refs/vendors/ARM/heads/morello)] aarch64: Tweak validity of label/symbol_ref addresses 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).