From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32035 invoked by alias); 15 Apr 2011 13:35:23 -0000 Received: (qmail 31953 invoked by uid 22791); 15 Apr 2011 13:35:21 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,TW_QE X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (94.185.240.25) by sourceware.org (qpsmtpd/0.43rc1) with SMTP; Fri, 15 Apr 2011 13:35:13 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 15 Apr 2011 14:35:10 +0100 Received: from [10.1.67.34] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Fri, 15 Apr 2011 14:35:07 +0100 Subject: Re: [PATCH, ARM] PR47855 Compute attr "length" for some thumb2 insns, 2/3 From: Richard Earnshaw To: Carrot Wei Cc: Ramana Radhakrishnan , gcc-patches@gcc.gnu.org In-Reply-To: References: <4D9EE8C7.6030709@linaro.org> Date: Fri, 15 Apr 2011 14:01:00 -0000 Message-Id: <1302874496.9717.198.camel@e102346-lin.cambridge.arm.com> Mime-Version: 1.0 X-MC-Unique: 111041514351002601 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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-04/txt/msg01194.txt.bz2 On Thu, 2011-04-14 at 21:19 +0800, Carrot Wei wrote: > On Fri, Apr 8, 2011 at 6:51 PM, Ramana Radhakrishnan > wrote: > > On 08/04/11 10:57, Carrot Wei wrote: > >> > >> Hi > >> > >> This is the second part of the fixing for > >> > >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D47855 > >> > >> This patch contains the length computation for insn patterns > >> "*arm_movqi_insn" > >> and "*arm_addsi3". Since the alternatives and encodings are much more > >> complex, > >> the attribute length is computed in separate C functions. >=20 > > Sorry, no. This is potentially a maintenance pain. It hardcodes alterna= tives > > from a pattern elsewhere in the C file. I don't like doing this unless = we > > have to with the sync primitives or with push_multi. In this case I'm n= ot > > convinced we need such functions in the .c file. > > > > Why can't we use the "enabled" attribute here with appropriate constrai= nts > > for everything other than the memory cases (even there we might be able= to > > invent some new constraints) ? > > > > Also a note about programming style. There are the helper macros like R= EG_P, > > CONST_INT_P and MEM_P which remove the necessity for checks like > > > > GET_CODE (x) =3D=3D y where y E { REG, CONST_INT, MEM} >=20 > Hi Ramana >=20 > As you suggested I created several new constraints, and use the > "enabled" attribute to split the current alternatives in this new > patch. It has been tested on arm qemu without regression. >=20 > thanks > Carrot Sorry, I don't think this approach can work. Certainly not with the way the compiler currently works, and especially for mov and add insns.=20 These instructions are only 2 bytes long if either: 1) They clobber the condition code register or 2) They occur inside an IT block. We can't tell either of these from the pattern, so you're underestimating the length of the instruction in some circumstances by claiming that they are only 2 bytes long. That /will/ lead to broken code someday. We can't add potential clobbers to mov and add patterns because that will break reload which relies on these patterns being simple-set insns with no added baggage. It *might* be possible to add clobbers to other operations, but that will then most-likely upset instruction scheduling (I think the scheduler treats two insns that clobber the same hard reg as being strongly ordered). Putting in the clobber too early will certainly affect cond-exec generation. In short, I'm not aware of a simple way to address this problem so that we get accurate length information, but minimal impact on other passes in the compiler. R.