From patchwork Fri Jan 14 10:36:46 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jie Zhang X-Patchwork-Id: 78879 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 0027DB7088 for ; Fri, 14 Jan 2011 21:37:10 +1100 (EST) Received: (qmail 12704 invoked by alias); 14 Jan 2011 10:37:07 -0000 Received: (qmail 12507 invoked by uid 22791); 14 Jan 2011 10:37:05 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 14 Jan 2011 10:36:59 +0000 Received: (qmail 21344 invoked from network); 14 Jan 2011 10:36:57 -0000 Received: from unknown (HELO ?192.168.0.102?) (jie@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Jan 2011 10:36:57 -0000 Message-ID: <4D30273E.2020400@codesourcery.com> Date: Fri, 14 Jan 2011 18:36:46 +0800 From: Jie Zhang User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101213 Lightning/1.0b2 Icedove/3.1.7 MIME-Version: 1.0 To: Paul Brook CC: gcc-patches@gcc.gnu.org Subject: Re: [ARM] Use r9 or r10 as the PIC register only when TARGET_32BIT References: <4CA0D4D5.50503@codesourcery.com> <201101121252.20665.paul@codesourcery.com> In-Reply-To: <201101121252.20665.paul@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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, * 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