From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9186 invoked by alias); 8 Jul 2011 02:32:10 -0000 Received: (qmail 9178 invoked by uid 22791); 8 Jul 2011 02:32:08 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Jul 2011 02:31:52 +0000 Received: from wpaz5.hot.corp.google.com (wpaz5.hot.corp.google.com [172.24.198.69]) by smtp-out.google.com with ESMTP id p682VogA018708 for ; Thu, 7 Jul 2011 19:31:51 -0700 Received: from qyk30 (qyk30.prod.google.com [10.241.83.158]) by wpaz5.hot.corp.google.com with ESMTP id p682VCJs023502 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 7 Jul 2011 19:31:44 -0700 Received: by qyk30 with SMTP id 30so62129qyk.6 for ; Thu, 07 Jul 2011 19:31:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.41.70 with SMTP id n6mr1152576qce.237.1310092299229; Thu, 07 Jul 2011 19:31:39 -0700 (PDT) Received: by 10.229.77.137 with HTTP; Thu, 7 Jul 2011 19:31:39 -0700 (PDT) In-Reply-To: <4DEE0089.50604@redhat.com> References: <4DEE0089.50604@redhat.com> Date: Fri, 08 Jul 2011 03:20:00 -0000 Message-ID: Subject: Re: [PATCH, ARM] PR47855 Compute attr length for thumb2 insns, 3/3 (issue4475042) From: Carrot Wei To: Nick Clifton , Richard Earnshaw Cc: GCC Patches Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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-07/txt/msg00579.txt.bz2 Thanks for the review. Richard, what's the situation of unaligned memory access and how does it conflict with this patch? thanks Carrot On Tue, Jun 7, 2011 at 6:42 PM, Nick Clifton wrote: > Hi Carrot, > >> 2011-05-06 =A0Guozhi Wei =A0 >> >> =A0 =A0 =A0 =A0PR target/47855 >> =A0 =A0 =A0 =A0* config/arm/thumb2.md (thumb2_movsi_insn): Add length ad= dtribute. >> =A0 =A0 =A0 =A0(thumb2_shiftsi3_short and peephole2): Remove 3-register = case. >> =A0 =A0 =A0 =A0(thumb2_cbz): Refine length computation. >> =A0 =A0 =A0 =A0(thumb2_cbnz): Likewise. > > Not approved - yet. > > The problem is the change to thumb2_movsi_insn. =A0You are still adding i= n the > support for the STM instruction despite the fact that Richard is still > researching how this will work with unaligned addresses. =A0Given the fact > that this change is not mentioned in the ChangeLog entry, I will assume t= hat > you intended to remove it and just forgot. > > I have no issues with the rest of your patch, so if you could submit an > updated patch I will be happy to review it again. > > One small point - when/if you do resubmit the STM part of the patch, you > could make the code slightly cleaner by enclosing it in curly parentheses, > thus avoiding the need to escape the double quote marks. =A0Ie: > > + =A0{ > + =A0switch (which_alternative) > + =A0 =A0{ > + =A0 =A0case 0: > + =A0 =A0case 1: > + =A0 =A0 =A0return "mov%?\t%0, %1"; > + > + =A0 =A0case 2: > + =A0 =A0 =A0return "mvn%?\t%0, #%B1"; > + > + =A0 =A0case 3: > + =A0 =A0 =A0return "movw%?\t%0, %1"; > + > + =A0 =A0case 4: > + =A0 =A0 =A0if (GET_CODE (XEXP (operands[1], 0)) =3D=3D POST_INC) > + =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 operands[1] =3D XEXP (XEXP (operands[1], 0), 0); > + =A0 =A0 =A0 =A0 return "ldm%(ia%)\t%1!, {%0}"; > + =A0 =A0 =A0 } > + =A0 =A0 /* Fall through. =A0*/ > + =A0 =A0case 5: > + =A0 =A0 =A0return "ldr%?\t%0, %1"; > + > + =A0 =A0case 6: > + =A0 =A0 =A0if (GET_CODE (XEXP (operands[0], 0)) =3D=3D POST_INC) > + =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 operands[0] =3D XEXP (XEXP (operands[0], 0), 0); > + =A0 =A0 =A0 =A0 return "stm%(ia%)\t%0!, {%1}"; > + =A0 =A0 =A0 } > + =A0 =A0 =A0/* Fall through. =A0*/ > + =A0 =A0case 7: > + =A0 =A0 =A0return "str%?\t%1, %0"; > + > + =A0 =A0default: > + =A0 =A0 =A0gcc_unreachable (); > + =A0 =A0} > + =A0} > > Cheers > =A0Nick > >