From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3856 invoked by alias); 21 Oct 2011 12:44:01 -0000 Received: (qmail 3847 invoked by uid 22791); 21 Oct 2011 12:43:59 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,TW_CP,TW_DM X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 21 Oct 2011 12:43:45 +0000 Received: by qam2 with SMTP id 2so3229535qam.20 for ; Fri, 21 Oct 2011 05:43:44 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.44.202 with SMTP id b10mr2787675qcf.241.1319201024582; Fri, 21 Oct 2011 05:43:44 -0700 (PDT) Received: by 10.229.215.207 with HTTP; Fri, 21 Oct 2011 05:43:44 -0700 (PDT) In-Reply-To: <4e83484c.03c7640a.2591.10bdSMTPIN_ADDED@mx.google.com> References: <4e83484c.03c7640a.2591.10bdSMTPIN_ADDED@mx.google.com> Date: Fri, 21 Oct 2011 12:52:00 -0000 Message-ID: Subject: Re: [RFA/ARM][Patch 01/02]: Thumb2 epilogue in RTL From: Ramana Radhakrishnan To: Sameera Deshpande Cc: gcc-patches@gcc.gnu.org, nickc@redhat.com, Richard Earnshaw , paul@codesourcery.com Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-10/txt/msg01967.txt.bz2 Hi Sameera, The comment about REG_FRAME_RELATED_EXPR vs REG_CFA_RESTORE from one of your later patches applies here as well. >diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >index 3162b30..f86a3e6 100644 >--- a/gcc/config/arm/arm.c >+++ b/gcc/config/arm/arm.c >@@ -8754,6 +8754,140 @@ neon_valid_immediate (rtx op, enum machine_mode mode, int inverse, > #undef CHECK > } > >+/* Return true if OP is a valid load multiple operation for MODE mode. >+ CONSECUTIVE is true if the registers in the operation must form >+ a consecutive sequence in the register bank. STACK_ONLY is true >+ if the base register must be the stack pointer. RETURN_PC is true >+ if value is to be loaded in PC. */ >+bool >+load_multiple_operation_p (rtx op, bool consecutive, enum machine_mode mode, >+ bool stack_only, bool return_pc) >+{ <...> snip >+ >+ /* If DFMode, we must be asking for consecutive, >+ since FLDMDD can only do consecutive regs. */ s/DFMode/DFmode s/FLDMDD/fldmdd (vldm.f64) Why are you differentiating on stack_only ? Does it really matter ? >+ gcc_assert ((mode != DFmode) || consecutive); >+ >+ /* Set up the increments and the regs per val based on the mode. */ >+ reg_increment = mode == DFmode ? 8 : 4; Can't you just get the reg_increment based on GET_MODE_SIZE (mode) ? >+ regs_per_val = mode == DFmode ? 2 : 1; >+ offset_adj = return_pc ? 1 : 0; >+ >+ if (count <= 1 >+ || GET_CODE (XVECEXP (op, 0, offset_adj)) != SET >+ || !REG_P (SET_DEST (XVECEXP (op, 0, offset_adj)))) >+ return false; >+ >+ /* Check to see if this might be a write-back. */ >+ if (GET_CODE (SET_SRC (elt = XVECEXP (op, 0, offset_adj))) == PLUS) >+ { >+ i++; >+ base = 1; >+ update = true; >+ >+ /* Now check it more carefully. */ >+ if (!REG_P (SET_DEST (elt)) >+ || !REG_P (XEXP (SET_SRC (elt), 0)) >+ || !CONST_INT_P (XEXP (SET_SRC (elt), 1)) >+ || INTVAL (XEXP (SET_SRC (elt), 1)) != >+ ((count - 1 - offset_adj) * reg_increment)) >+ return false; A comment here explaining that you are checking for the increment amount being sane would be good. >+ >+ /* Check the nature of the base_register being written to. */ >+ if (stack_only && (REGNO (SET_DEST (elt)) != SP_REGNUM)) >+ return false; >+ } >+ >+ i = i + offset_adj; >+ base = base + offset_adj; >+ /* Perform a quick check so we don't blow up below. */ >+ if (GET_CODE (XVECEXP (op, 0, i - 1)) != SET >+ || !REG_P (SET_DEST (XVECEXP (op, 0, i - 1))) >+ || !MEM_P (SET_SRC (XVECEXP (op, 0, i - 1)))) >+ return false; >+ >+ /* If only one reg being loaded, success depends on the type: >+ FLDMDD can do just one reg, LDM must do at least two. */ Hmmm isn't this true of only LDM's in Thumb state ? Though it could be argued that this patch is only T2 epilogues. >+ if (count <= i) >+ return mode == DFmode ? true : false; Again a comment here would be useful. >+ >+ first_dest_regno = REGNO (SET_DEST (XVECEXP (op, 0, i - 1))); >+ dest_regno = first_dest_regno; >+ >+ src_addr = XEXP (SET_SRC (XVECEXP (op, 0, i - 1)), 0); >+ >+ if (GET_CODE (src_addr) == PLUS) >+ { >+ if (!CONST_INT_P (XEXP (src_addr, 1))) >+ return false; Watch out for the indentation of the return. <...snip> >+) >+ >+(define_insn "*floating_point_pop_multiple_with_stack_update" s/floating_point/vfp >+ [(match_parallel 0 "load_multiple_operation_stack_fp" >+ [(set (match_operand:SI 1 "s_register_operand" "=k") >+ (plus:SI (match_operand:SI 2 "s_register_operand" "1") >+ (match_operand:SI 3 "const_int_operand" "I"))) >+ (set (match_operand:DF 4 "arm_hard_register_operand" "") >+ (mem:DF (match_dup 2)))])] >+ "TARGET_THUMB2" && TARGET_HARD_FLOAT && TARGET_VFP >+ "* >+ { >+ int num_regs = XVECLEN (operands[0], 0); >+ static const struct { const char *const name; } table[] >+ = { {\"d0\"}, {\"d1\"}, {\"d2\"}, {\"d3\"}, >+ {\"d4\"}, {\"d5\"}, {\"d6\"}, {\"d7\"}, >+ {\"d8\"}, {\"d9\"}, {\"d10\"}, {\"d11\"}, >+ {\"d12\"}, {\"d13\"}, {\"d14\"}, {\"d15\"}, >+ {\"d16\"}, {\"d17\"}, {\"d18\"}, {\"d19\"}, >+ {\"d20\"}, {\"d21\"}, {\"d22\"}, {\"d23\"}, >+ {\"d24\"}, {\"d25\"}, {\"d26\"}, {\"d27\"}, >+ {\"d28\"}, {\"d29\"}, {\"d30\"}, {\"d31\"} }; >+ int i; >+ char pattern[100]; >+ strcpy (pattern, \"fldmfdd\\t\"); >+ strcat (pattern, >+ reg_names[REGNO (SET_DEST (XVECEXP (operands[0], 0, 0)))]); >+ strcat (pattern, \"!, {\"); >+ strcat (pattern, table[(REGNO (XEXP (XVECEXP (operands[0], 0, 1), 0)) >+ - FIRST_VFP_REGNUM) / 2].name); Can't you reuse names from arm.h and avoid the table here ? >+ for (i = 2; i < num_regs; i++) >+ { >+ strcat (pattern, \", %|\"); >+ strcat (pattern, table[(REGNO (XEXP (XVECEXP (operands[0], 0, i), 0)) >+ - FIRST_VFP_REGNUM) / 2].name); >+ } Can't you use fldmfdd {reg_lo-reg_hi} instead of enumerating all the registers here. >+ strcat (pattern, \"}\"); >+ output_asm_insn (pattern, operands); >+ return \"\"; >+ } >+ " >+ [(set_attr "type" "load4")] Ramana