From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11374 invoked by alias); 1 Jun 2011 20:21:16 -0000 Received: (qmail 11361 invoked by uid 22791); 1 Jun 2011 20:21:15 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 01 Jun 2011 20:20:59 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 07706CB02C5; Wed, 1 Jun 2011 22:20:58 +0200 (CEST) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6avQnR2k2dNN; Wed, 1 Jun 2011 22:20:55 +0200 (CEST) Received: from [192.168.1.2] (bon31-9-83-155-120-49.fbx.proxad.net [83.155.120.49]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id DC35ACB0223; Wed, 1 Jun 2011 22:20:54 +0200 (CEST) From: Eric Botcazou To: Richard Sandiford Subject: Re: PR 49145: Another (zero_extend (const_int ...)) in combine Date: Wed, 01 Jun 2011 20:21:00 -0000 User-Agent: KMail/1.9.9 Cc: gcc-patches@gcc.gnu.org References: <877h9ahflh.fsf@firetop.home> In-Reply-To: <877h9ahflh.fsf@firetop.home> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <201106012220.49874.ebotcazou@adacore.com> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit 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-06/txt/msg00099.txt.bz2 > I've included the subreg handling as well as the zero_extend handling, > even though make_compound_operation already handles that case correctly. > However, I've cowardly not removed the known_cond subreg handling: > > else if (code == SUBREG) > { > enum machine_mode inner_mode = GET_MODE (SUBREG_REG (x)); > rtx new_rtx, r = known_cond (SUBREG_REG (x), cond, reg, val); > > if (SUBREG_REG (x) != r) > { > /* We must simplify subreg here, before we lose track of the > original inner_mode. */ > new_rtx = simplify_subreg (GET_MODE (x), r, > inner_mode, SUBREG_BYTE (x)); > if (new_rtx) > return new_rtx; > else > SUBST (SUBREG_REG (x), r); > } > > return x; > } > > The new function would also handle the case described in the comment. > However, I was afraid that we might rely on simplify_subreg being > used for all simplified operands here, while also relying on it only > being used for constants in subst. (This comes from a general > fear that combine is special in the way that it handles compound > operations. Calling the generic simplification routines too often > could cause us to turn combine's preferred representation into > the representation normally used elsewhere.) Frankly, I'm not convinced that the benefits are worth the potential hassle in this case. As you mentioned, the 3 functions already have their own handling for SUBREGs, slightly different from each other, so factoring it into a common form isn't trivial; now, if you do it and don't defer to the new function for the entire handling after that, you don't really simplify the code. So we're essentially (see below) left with the ZERO_EXTEND case and I'd add the handful of missing lines to make_compound_operation and be done with it. > The known_cond code has the comment: > > /* We don't have to handle SIGN_EXTEND here, because even in the > case of replacing something with a modeless CONST_INT, a > CONST_INT is already (supposed to be) a valid sign extension for > its narrower mode, which implies it's already properly > sign-extended for the wider mode. Now, for ZERO_EXTEND, the > story is different. */ > > While that is true, I don't see any point in going out of our way > _not_ to handle sign_extend in the same way. Or indeed all unary > operators. Testing UNARY_P seems justified on the basis that > simplify_unary_operation requires the mode of the inner operand, > and that losing that mode without giving simplify_unary_operation > a chance could therefore lead to wrong results. I'd rather not > see us hard-code the knowledge of which operators actually care. SUBREG and ZERO_EXTEND of CONST_INTs are treated somewhat specially in the entire file, see for example do_SUBST. This isn't the case for other unary operators, presumably because this isn't really necessary here. So I'm not convinced that such a generalization is really a good thing in this case. -- Eric Botcazou