From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21944 invoked by alias); 31 Mar 2011 15:57:59 -0000 Received: (qmail 21927 invoked by uid 22791); 31 Mar 2011 15:57:58 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 31 Mar 2011 15:57:53 +0000 Received: by wwf26 with SMTP id 26so2903401wwf.8 for ; Thu, 31 Mar 2011 08:57:52 -0700 (PDT) Received: by 10.227.110.147 with SMTP id n19mr2823467wbp.51.1301587072250; Thu, 31 Mar 2011 08:57:52 -0700 (PDT) Received: from [192.168.32.37] (fw-lnat.cambridge.arm.com [217.140.96.63]) by mx.google.com with ESMTPS id g7sm734418wby.31.2011.03.31.08.57.51 (version=SSLv3 cipher=OTHER); Thu, 31 Mar 2011 08:57:51 -0700 (PDT) Message-ID: <4D94A47C.5070305@linaro.org> Date: Thu, 31 Mar 2011 16:04:00 -0000 From: Ramana Radhakrishnan User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8 MIME-Version: 1.0 To: Carrot Wei CC: gcc-patches@gcc.gnu.org Subject: Re: [PATCH, ARM] PR47855 Compute attr "length" for some thumb2 insns References: <4D925EA7.2080102@linaro.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-03/txt/msg02221.txt.bz2 >> Hi Carrot, >> >> >> How about adding an alternative only enabled for T2 that uses the `l' >> constraint and inventing new constraints for some of the constant values >> that are valid for 16 bit instructions since the `I' and `L' constraints >> have different meanings depending on whether TARGET_32BIT is valid or not ? >> We could then set the value of the length attribute accordingly. I don't >> think we can change the meaning of the I and L constraints very easily given >> the amount of churn that might be needed >> > You are right. Now the logic is much clearer by splitting the constraints. Sorry I wasn't too clear . What I meant was to use the "enabled" trick and enable this only when Thumb2 is enabled in the compiler. So what you could do instead is add the alternative and then use the arch attribute to enable this only for Thumb2. i.e. >> (define_insn "*arm_cmpsi_insn" >> [(set (reg:CC CC_REGNUM) >> - (compare:CC (match_operand:SI 0 "s_register_operand" "r,r") >> - (match_operand:SI 1 "arm_add_operand" "rI,L")))] >> + (compare:CC (match_operand:SI 0 "s_register_operand" "l,r,r,r") >> + (match_operand:SI 1 "arm_add_operand" "Py,r,I,L")))] >> "TARGET_32BIT" >> "@ >> cmp%?\\t%0, %1 >> + cmp%?\\t%0, %1 >> + cmp%?\\t%0, %1 >> cmn%?\\t%0, #%n1" >> - [(set_attr "conds" "set")] >> + [(set_attr "conds" "set") (set_attr "arch" "t2, any") (set_attr "length" "2, 4")] Does that look any better ? That way you can now safely add options on a per arch basis and reduce the amount of complexity for some of these length calculations. >> >> I don't think this and the other conditional branch instruction >> changes are correct. This could end up being the last instruction in an IT >> block and will automatically end up getting the 32 bit encoding and end up >> causing trouble with the length calculations. Remember the 16 bit encoding >> for the conditional instruction can't be used as the last instruction in an >> IT block. >> > According to arm architecture reference manual, chapter A8.6.16, neither 16 bit > nor 32 bit conditional branch can be used in IT block. Only unconditional branch > can be used as the last instruction in IT block, and it isn't related > to instruction > length. So we don't need to care about branch instruction encoding in IT block. Yes I think you are right in this case. The reason I think I misinterpreted this was ofcourse the fact that conditional branches as the last instruction in the ARM-ARM. I don't know why I misread it last night. cheers Ramana