From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16616 invoked by alias); 23 Aug 2011 09:06:34 -0000 Received: (qmail 16581 invoked by uid 22791); 23 Aug 2011 09:06:32 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_TX 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, 23 Aug 2011 09:06:15 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7N95q7a016456 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 23 Aug 2011 05:05:52 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p7N95oUn006579 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Aug 2011 05:05:51 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (localhost.localdomain [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id p7N95o7R009818; Tue, 23 Aug 2011 11:05:50 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id p7N95nPh009816; Tue, 23 Aug 2011 11:05:49 +0200 Date: Tue, 23 Aug 2011 10:07:00 -0000 From: Jakub Jelinek To: Bernd Schmidt , Eric Botcazou , Richard Sandiford Cc: Richard Henderson , GCC Patches Subject: Re: Add __builtin_clrsb, similar to clz/ctz Message-ID: <20110823090549.GC2687@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek References: <4DF9FA9A.8040505@codesourcery.com> <4DFA2E85.2030601@redhat.com> <4DFFA1AE.7070405@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4DFFA1AE.7070405@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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-08/txt/msg01854.txt.bz2 On Mon, Jun 20, 2011 at 09:38:22PM +0200, Bernd Schmidt wrote: > D'oh. Blackfin has a (clrsb:HI (operand:SI)) instruction, so adding this > showed a problem with some of the existing simplify_const_unop cases: > for ffs/clz/ctz/clrsb/parity/popcount, we should look at the mode of the > operand, rather than the mode of the operation. This limits what we can > do in that function, since op_mode is sometimes VOIDmode - we really > should add builtin folders for these at some point. > * simplify-rtx.c (simplify_const_unary_operation): Likewise. > Use op_mode rather than mode when optimizing ffs, clz, ctz, parity > and popcount. This change is IMHO wrong, see e.g. PR50161 where we have (subreg:SI (popcount:DI (const_int -1))). This is supposed to yield 64, but with your changes it yields 128 - the op_mode here is VOIDmode, so the first if that used to handle it is no longer used, but as width is <= 2 * HOST_BITS_PER_WIDE_INT, it is treated as TImode constant. IMHO best would be just to mandate that for these unary ops like FFS, CLZ, CLRSB, CTZ, POPCOUNT, PARITY, BSWAP the operand has the same mode (or VOIDmode) as the unary rtx and that the operation is being carried in the unop's mode, it shouldn't be hard to adjust the few *.md patterns (mainly in avr.md, bfin.md). I think it is bad enough that ZERO_EXTEND must not have CONST_INT argument, making CONST_INT undefined also for all these unary ops is unnecessary. I think for NEG/NOT we already have such a guarantee (and thus your change pessimizes it anyway). avr.md/bfin.md etc. can use (subreg:HI (popcount:SI (match_operand:SI ...))) (or (zero_extend:HI (popcount:QI (match_operand:QI ...))) and similar. Or the /* We can do some operations on integer CONST_DOUBLEs. Also allow for a DImode operation on a CONST_INT. */ else if (GET_MODE (op) == VOIDmode && width <= HOST_BITS_PER_WIDE_INT * 2 && (GET_CODE (op) == CONST_DOUBLE || CONST_INT_P (op))) case would need to change too to test that op_width == HOST_BITS_PER_WIDE_INT * 2 (but then, it would again pessimize at least NEG/NOT/ABS that are defined sanely). But we'd also need to change many other places, e.g. cse_process_notes_1, that currently special case ZERO_EXTEND/SUBREG (and sometimes SIGN_EXTEND) and pessimize them because those rtxes aren't allowed to have VOIDmode arguments. cse_process_notes_1 perhaps could be changed for VOIDmode new_rtx to try to simplify_replace_rtx it... Jakub