diff mbox

[AArch64] Allow symbol+offset even if not being used for memory access

Message ID 504E3877.90003@redhat.com
State New
Headers show

Commit Message

Richard Henderson Sept. 10, 2012, 6:59 p.m. UTC 
On 09/10/2012 09:09 AM, Ian Bolton wrote:
>> Can you send me the test case you were looking at for this?
> 
> See attached.  (Most of it is superfluous, but the point is that
> we are not using the address to do a memory access.)

Ok.  Having dug a bit deeper I think the main problem is that you're
working against yourself by not handling this pattern right from the
beginning.  You have split the address incorrectly to begin and are
now trying to recover after the fact.

The following patch seems to do the trick for me, producing

> (insn 6 5 7 (set (reg:DI 81)
>         (high:DI (const:DI (plus:DI (symbol_ref:DI ("arr") [flags 0x80]  <var_decl 0x7f9bae1105f0 arr>)
>                     (const_int 12 [0xc]))))) z.c:8 -1
>      (nil))
> 
> (insn 7 6 8 (set (reg:DI 80)
>         (lo_sum:DI (reg:DI 81)
>             (const:DI (plus:DI (symbol_ref:DI ("arr") [flags 0x80]  <var_decl 0x7f9bae1105f0 arr>)
>                     (const_int 12 [0xc]))))) z.c:8 -1
>      (expr_list:REG_EQUAL (const:DI (plus:DI (symbol_ref:DI ("arr") [flags 0x80]  <var_decl 0x7f9bae1105f0 arr>)
>                 (const_int 12 [0xc])))
>         (nil)))

right from the .150.expand dump.

I'll leave it to you to fully regression test and commit the patch
as appropriate.  ;-)


r~

Comments

Ian Bolton Sept. 25, 2012, 1:05 p.m. UTC | #1 
> Ok.  Having dug a bit deeper I think the main problem is that you're
> working against yourself by not handling this pattern right from the
> beginning.  You have split the address incorrectly to begin and are
> now trying to recover after the fact.
> 
> The following patch seems to do the trick for me, producing
> 
> > (insn 6 5 7 (set (reg:DI 81)
> >         (high:DI (const:DI (plus:DI (symbol_ref:DI ("arr") [flags
> 0x80]  <var_decl 0x7f9bae1105f0 arr>)
> >                     (const_int 12 [0xc]))))) z.c:8 -1
> >      (nil))
> >
> > (insn 7 6 8 (set (reg:DI 80)
> >         (lo_sum:DI (reg:DI 81)
> >             (const:DI (plus:DI (symbol_ref:DI ("arr") [flags 0x80]
> <var_decl 0x7f9bae1105f0 arr>)
> >                     (const_int 12 [0xc]))))) z.c:8 -1
> >      (expr_list:REG_EQUAL (const:DI (plus:DI (symbol_ref:DI ("arr")
> [flags 0x80]  <var_decl 0x7f9bae1105f0 arr>)
> >                 (const_int 12 [0xc])))
> >         (nil)))
> 
> right from the .150.expand dump.
> 
> I'll leave it to you to fully regression test and commit the patch
> as appropriate.  ;-)
> 

Thanks so much for this, Richard.

I have prepared a new patch heavily based off yours, which really
demands its own new email trail, so I shall make a fresh post.

Cheers,
Ian
diff mbox

Patch

Index: aarch64.md
===================================================================
--- aarch64.md	(revision 191152)
+++ aarch64.md	(working copy)
@@ -2840,7 +2840,7 @@ 
 	(lo_sum:DI (match_operand:DI 1 "register_operand" "r")
 		   (match_operand 2 "aarch64_valid_symref" "S")))]
   ""
-  "add\\t%0, %1, :lo12:%2"
+  "add\\t%0, %1, :lo12:%a2"
   [(set_attr "v8type" "alu")
    (set_attr "mode" "DI")]
 
Index: aarch64.c
===================================================================
--- aarch64.c	(revision 191152)
+++ aarch64.c	(working copy)
@@ -652,43 +652,57 @@ 
   unsigned HOST_WIDE_INT val;
   bool subtargets;
   rtx subtarget;
-  rtx base, offset;
   int one_match, zero_match;
 
   gcc_assert (mode == SImode || mode == DImode);
 
-  /* If we have (const (plus symbol offset)), and that expression cannot
-     be forced into memory, load the symbol first and add in the offset.  */
-  split_const (imm, &base, &offset);
-  if (offset != const0_rtx
-      && (targetm.cannot_force_const_mem (mode, imm)
-	  || (can_create_pseudo_p ())))
+  /* Check on what type of symbol it is.  */
+  if (GET_CODE (imm) == SYMBOL_REF
+      || GET_CODE (imm) == LABEL_REF
+      || GET_CODE (imm) == CONST)
     {
-      base = aarch64_force_temporary (dest, base);
-      aarch64_emit_move (dest, aarch64_add_offset (mode, NULL, base, INTVAL (offset)));
-      return;
-    }
+      rtx mem, base, offset;
+      enum aarch64_symbol_type sty;
 
-  /* Check on what type of symbol it is.  */
-  if (GET_CODE (base) == SYMBOL_REF || GET_CODE (base) == LABEL_REF)
-    {
-      rtx mem;
-      switch (aarch64_classify_symbol (base, SYMBOL_CONTEXT_ADR))
+      /* If we have (const (plus symbol offset)), separate out the offset
+	 before we start classifying the symbol.  */
+      split_const (imm, &base, &offset);
+
+      sty = aarch64_classify_symbol (base, SYMBOL_CONTEXT_ADR);
+      switch (sty)
 	{
 	case SYMBOL_FORCE_TO_MEM:
-	  mem  = force_const_mem (mode, imm);
+	  if (offset != const0_rtx
+	      && targetm.cannot_force_const_mem (mode, imm))
+	    {
+	      gcc_assert(can_create_pseudo_p ());
+	      base = aarch64_force_temporary (dest, base);
+	      base = aarch64_add_offset (mode, NULL, base, INTVAL (offset));
+	      aarch64_emit_move (dest, base);
+	      return;
+	    }
+	  mem = force_const_mem (mode, imm);
 	  gcc_assert (mem);
 	  emit_insn (gen_rtx_SET (VOIDmode, dest, mem));
 	  return;
 
-    case SYMBOL_SMALL_TLSGD:
-    case SYMBOL_SMALL_TLSDESC:
-    case SYMBOL_SMALL_GOTTPREL:
-    case SYMBOL_SMALL_TPREL:
+        case SYMBOL_SMALL_TLSGD:
+        case SYMBOL_SMALL_TLSDESC:
+        case SYMBOL_SMALL_GOTTPREL:
 	case SYMBOL_SMALL_GOT:
+	  if (offset != const0_rtx)
+	    {
+	      gcc_assert(can_create_pseudo_p ());
+	      base = aarch64_force_temporary (dest, base);
+	      base = aarch64_add_offset (mode, NULL, base, INTVAL (offset));
+	      aarch64_emit_move (dest, base);
+	      return;
+	    }
+	  /* FALLTHRU */
+
+        case SYMBOL_SMALL_TPREL:
 	case SYMBOL_SMALL_ABSOLUTE:
-	  aarch64_load_symref_appropriately
-	    (dest, imm, aarch64_classify_symbol (base, SYMBOL_CONTEXT_ADR));
+	  aarch64_load_symref_appropriately (dest, imm, sty);
 	  return;
 
 	default:
@@ -696,7 +710,7 @@ 
 	}
     }
 
-  if ((CONST_INT_P (imm) && aarch64_move_imm (INTVAL (imm), mode)))
+  if (CONST_INT_P (imm) && aarch64_move_imm (INTVAL (imm), mode))
     {
       emit_insn (gen_rtx_SET (VOIDmode, dest, imm));
       return;