From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29781 invoked by alias); 29 Mar 2011 22:00:35 -0000 Received: (qmail 29770 invoked by uid 22791); 29 Mar 2011 22:00:32 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 29 Mar 2011 22:00:19 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2TM0I3P026927 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 29 Mar 2011 18:00:19 -0400 Received: from anchor.twiddle.home (ovpn-113-123.phx2.redhat.com [10.3.113.123]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p2TM0HRl013138; Tue, 29 Mar 2011 18:00:18 -0400 Message-ID: <4D925671.5030402@redhat.com> Date: Tue, 29 Mar 2011 22:14:00 -0000 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110307 Fedora/3.1.9-0.39.b3pre.fc14 Thunderbird/3.1.9 MIME-Version: 1.0 To: Mikael Pettersson , Andreas Krebbel , gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Subject: Re: Cleaning up expand optabs code References: <4D825EF5.1010307@redhat.com> <87wrju28hn.fsf@firetop.home> <4D87A69B.90704@redhat.com> <4D88E10A.8080005@redhat.com> <4D8B32A9.9050309@linux.vnet.ibm.com> <19857.52792.228466.501327@pilspetsen.it.uu.se> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 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/msg02035.txt.bz2 On 03/29/2011 06:21 AM, Richard Sandiford wrote: > - enum machine_mode mode0 = insn_data[(int) icode].operand[1].mode; > - enum machine_mode mode1 = insn_data[(int) icode].operand[2].mode; > - enum machine_mode tmp_mode; > + enum machine_mode xmode0 = insn_data[(int) icode].operand[1].mode; > + enum machine_mode xmode1 = insn_data[(int) icode].operand[2].mode; > + enum machine_mode mode0, mode1, tmp_mode; ... > - if (GET_MODE (xop0) != mode0 && mode0 != VOIDmode) > - xop0 = convert_modes (mode0, > - GET_MODE (xop0) != VOIDmode > - ? GET_MODE (xop0) > - : mode, > - xop0, unsignedp); > - > - if (GET_MODE (xop1) != mode1 && mode1 != VOIDmode) > - xop1 = convert_modes (mode1, > - GET_MODE (xop1) != VOIDmode > - ? GET_MODE (xop1) > - : mode, > - xop1, unsignedp); > + mode0 = GET_MODE (xop0) != VOIDmode ? GET_MODE (xop0) : mode; > + if (xmode0 != VOIDmode && xmode0 != mode0) > + { > + xop0 = convert_modes (xmode0, mode0, xop0, unsignedp); > + mode0 = xmode0; > + } > + > + mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > + if (xmode1 != VOIDmode && xmode1 != mode1) > + { > + xop1 = convert_modes (xmode1, mode1, xop1, unsignedp); > + mode1 = xmode1; > + } This smells like a target bug, but I can't quite put my finger on exactly what's wrong, and I haven't debugged the original. The backend has said xmode[01] is what it expects. The only reason you wouldn't write xmode[01] in the create_input_operand line is if xmode[01] are VOIDmode. That seems like mistake number one, particularly for a binop, but I'll accept that for the nonce. Which pattern triggered the problem in the target? Then we've got the conditionalization in the init of mode[01], which is presumably to handle CONST_INT as an input. What about something like xmode0 = insn_data... if (xmode0 == VOIDmode) xmode0 = mode; mode0 = GET_MODE (xop0); if (mode0 == VOIDmode) mode0 = mode; if (xmode0 != mode0) convert_modes create_input_operand(..., xmode0) ? That seems more obvious than what you have. And I *think* it should produce the same results. If it doesn't, then this whole block of code needs a lot more explanation. r~