Patchwork [ARM] Use r9 or r10 as the PIC register only when TARGET_32BIT

login
register
mail settings
Submitter Jie Zhang
Date Jan. 14, 2011, 10:36 a.m.
Message ID <4D30273E.2020400@codesourcery.com>
Download mbox | patch
Permalink /patch/78879/
State New
Headers show

Comments

Jie Zhang - Jan. 14, 2011, 10:36 a.m.
On 01/12/2011 08:52 PM, Paul Brook wrote:
>> The original line of the code is several years old. It might have been
>> broken for a long time. It could be no one use -fpic on ARM uClinux or
>> -msingle-pic-base on other arm targets. Or I missed something.
>
> AFAIK arm-uclinux only has a relatively small number of users and I'd bet most
> of those don't use PIC[1].  It's entirely possible you're the first person to
> notice and care that -mthumb -fPIC broke[2].  -msingle-pic-base is an ABI
> changing option, so specifying it manually (rather than inheriting your OS
> defaults) almost certainly means you're doing something else wrong.
>
>> Does this patch look OK?
>
> No.
>
> The whole point of -msingle-pic-base is to use a fixed base register for data
> addressing (in EABI terms, this is SB-relative addressing).  The pic base
> register is specified by the ABI.  The fact that legacy targets can't decide
> whether to use r9 or r10 is probably broken, but I find it hard to care about
> such targets.
>
> i.e. we really do want to use r9 as the pic base register.  You need to fix
> whatever broke, rather than unilaterally picking a more convenient register.
>
Thanks for review. I thought THUMB1 code could not use r9. Apparently I 
was wrong. So I took a closer look at this issue. This time I found it 
was a recent regression caused by r162595 several months ago. That 
revision changed ARM port to generate pic address by using 
calculate_pic_address pattern, but didn't change 
thumb1_legitimate_address_p to accept the new address for THUMB1. The 
second issue is the split pattern does not always generate valid address 
for THUMB1. This patch should fix both issues. Regression tested on our 
internal 4.5 branch for arm-uclinuxeabi for gcc, g++ and libstdc++. The 
following FAILs are fixed:

FAIL: gcc.target/arm/pr42495.c (internal compiler error)
FAIL: gcc.target/arm/pr42495.c (test for excess errors)
FAIL: gcc.target/arm/pr42495.c scan-rtl-dump hoist "PRE/HOIST: end of bb 
.* copying expression": dump file does not exist
FAIL: gcc.target/arm/pr42574.c (internal compiler error)
FAIL: gcc.target/arm/pr42574.c (test for excess errors)
ERROR: gcc.target/arm/pr42574.c: error executing dg-final: couldn't open 
"pr42574.s": no such file or directory
UNRESOLVED: gcc.target/arm/pr42574.c: error executing dg-final: couldn't 
open "pr42574.s": no such file or directory

I'm lack of infrastructure to test it against FSF trunk.

OK?

Regards,

Patch


	* config/arm/arm.c (thumb1_index_register_rtx_p): Remove
	declaration.
	(thumb1_index_register_rtx_p): Make global.
	(thumb1_legitimate_address_p): allow the address generated
	by calculate_pic_address pattern when !strict_p.
	* config/arm/arm-protos.h (thumb1_index_register_rtx_p): Declare.
	* config/arm/arm.md (define_split for calculate_pic_address):
	Generate valid memory address when TARGET_THUMB1.

Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 168776)
+++ config/arm/arm.c	(working copy)
@@ -77,7 +77,6 @@  static int thumb2_legitimate_index_p (en
 static int thumb1_base_register_rtx_p (rtx, enum machine_mode, int);
 static rtx arm_legitimize_address (rtx, rtx, enum machine_mode);
 static rtx thumb_legitimize_address (rtx, rtx, enum machine_mode);
-inline static int thumb1_index_register_rtx_p (rtx, int);
 static bool arm_legitimate_address_p (enum machine_mode, rtx, bool);
 static int thumb_far_jump_used_p (void);
 static bool thumb_force_lr_save (void);
@@ -5889,7 +5888,7 @@  thumb1_base_register_rtx_p (rtx x, enum
 
 /* Return nonzero if x is a legitimate index register.  This is the case
    for any base register that can access a QImode object.  */
-inline static int
+int
 thumb1_index_register_rtx_p (rtx x, int strict_p)
 {
   return thumb1_base_register_rtx_p (x, QImode, strict_p);
@@ -5965,6 +5964,17 @@  thumb1_legitimate_address_p (enum machin
 	      || (!strict_p && will_be_in_index_register (XEXP (x, 1)))))
 	return 1;
 
+      /* We allow the address generated by "calculate_pic_address"
+	 pattern when !strict_p.  That pattern will be split into one
+	 of the pic_load_addr_* patterns and a move later.  */
+      else if (GET_MODE_SIZE (mode) <= 4
+	       && !strict_p
+	       && XEXP (x, 0) != frame_pointer_rtx
+	       && arm_pic_register != INVALID_REGNUM
+	       && REGNO (XEXP (x, 0)) == arm_pic_register
+	       && will_be_in_index_register (XEXP (x, 1)))
+	return 1;
+
       /* REG+const has 5-7 bit offset for non-SP registers.  */
       else if ((thumb1_index_register_rtx_p (XEXP (x, 0), strict_p)
 		|| XEXP (x, 0) == arg_pointer_rtx)
Index: config/arm/arm-protos.h
===================================================================
--- config/arm/arm-protos.h	(revision 168776)
+++ config/arm/arm-protos.h	(working copy)
@@ -49,6 +49,7 @@  extern int const_ok_for_arm (HOST_WIDE_I
 extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
 			       HOST_WIDE_INT, rtx, rtx, int);
 extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *);
+extern int thumb1_index_register_rtx_p (rtx, int);
 extern int legitimate_pic_operand_p (rtx);
 extern rtx legitimize_pic_address (rtx, enum machine_mode, rtx);
 extern rtx legitimize_tls_address (rtx, rtx);
Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 168776)
+++ config/arm/arm.md	(working copy)
@@ -5255,10 +5255,23 @@  (define_split
 			 (unspec:SI [(match_operand:SI 2 "" "")]
 				    UNSPEC_PIC_SYM))))]
   "flag_pic"
-  [(set (match_dup 3) (unspec:SI [(match_dup 2)] UNSPEC_PIC_SYM))
-   (set (match_dup 0) (mem:SI (plus:SI (match_dup 1) (match_dup 3))))]
-  "operands[3] = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0];"
-)
+  [(set (match_dup 0) (mem:SI (match_dup 3)))]
+  "
+{
+  operands[3] = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0];
+  emit_insn (gen_movsi (operands[3], gen_rtx_UNSPEC (SImode,
+						     gen_rtvec (1, operands[2]),
+						     UNSPEC_PIC_SYM)));
+  if (TARGET_32BIT
+      || (TARGET_THUMB1
+	  && thumb1_index_register_rtx_p (operands[1], 0)
+	  && (can_create_pseudo_p ()
+	      || thumb1_index_register_rtx_p (operands[0], 0))))
+    operands[3] = gen_rtx_PLUS (SImode, operands[1], operands[3]);
+  else
+    emit_insn (gen_movsi (operands[3],
+			  gen_rtx_PLUS (SImode, operands[1], operands[3])));
+}")
 
 ;; The rather odd constraints on the following are to force reload to leave
 ;; the insn alone, and to force the minipool generation pass to then move