From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11966 invoked by alias); 25 Jun 2014 09:36:25 -0000 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 Received: (qmail 11957 invoked by uid 89); 25 Jun 2014 09:36:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f179.google.com Received: from mail-wi0-f179.google.com (HELO mail-wi0-f179.google.com) (209.85.212.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 25 Jun 2014 09:36:23 +0000 Received: by mail-wi0-f179.google.com with SMTP id cc10so2141806wib.0 for ; Wed, 25 Jun 2014 02:36:20 -0700 (PDT) X-Received: by 10.180.21.200 with SMTP id x8mr5432639wie.70.1403688980125; Wed, 25 Jun 2014 02:36:20 -0700 (PDT) Received: from localhost ([2.26.169.52]) by mx.google.com with ESMTPSA id ft20sm10352061wic.14.2014.06.25.02.36.19 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Jun 2014 02:36:19 -0700 (PDT) From: Richard Sandiford To: Oleg Endo Mail-Followup-To: Oleg Endo ,David Malcolm , Steven Bosscher , GCC Patches , rdsandiford@googlemail.com Cc: David Malcolm , Steven Bosscher , GCC Patches Subject: Re: Instructions vs Expressions in the backend (was Re: RFA: Rework FOR_BB_INSNS iterators) References: <87vbscppva.fsf@talisman.default> <87d2ehq3p6.fsf@talisman.default> <1403549659.16446.43.camel@surprise> <1403555889.27443.121.camel@yam-132-YW-E178-FTW> Date: Wed, 25 Jun 2014 09:36:00 -0000 In-Reply-To: <1403555889.27443.121.camel@yam-132-YW-E178-FTW> (Oleg Endo's message of "Mon, 23 Jun 2014 22:38:09 +0200") Message-ID: <87bnthgwku.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2014-06/txt/msg01991.txt.bz2 Oleg Endo writes: > Personally, I'd like to see usage of standard STL-like iterator usage. > I've proposed something for edge_iterator a while ago, but people don't > seem very fond of it. See also > https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01129.html > > Have you also considered passing the new rtx_* types by value or > reference instead of pointer? A long time ago I've quickly put together > a class 'rtxx' which was just a pointer wrapper for the rtx_def* > (basically the same as I proposed for edge_iterator). > I've converted the SH backend code to use it just to see what it would > look like. The conversion itself was pretty straight forward -- just > replace 'rtx' with 'rtxx'. Appropriate conversion > operators/constructors in 'class rtxx' made both interchangeable and > allowed co-existence of both and thus step-by-step conversion of the > code base. > Another advantage of passing around by value/ref is that it allows > operator overloading. One use case could be instead of: > > if (MEM_P (XEXP (x, 0))) > *total = address_cost (XEXP (XEXP (x, 0), 0), > GET_MODE (XEXP (x, 0)), > MEM_ADDR_SPACE (XEXP (x, 0)), true); > > > something like that (overloading operator[]): > if (x[0] == rtx_mem::type) > *total = address_cost (x[0][0], x[0].mode (), > x[0].mem_addr_space (), true); > > ... where rtx_mem::type would be some type for which 'rtxx' (or whatever > the name of the base class is) would provide the according operator > ==, != overloads. I think this is an example of another problem with gcc coding style: that we're far too afraid of temporary variables. In David's scheme I think this would be: if (rtx_mem *mem = as_a (XEXP (x, 0))) *total = address_cost (XEXP (mem, 0), GET_MODE (mem), MEM_ADDR_SPACE (mem), true); which with members would become: if (rtx_mem *mem = as_a (...)) *total = address_cost (mem->address (), mem->mode (), mem->address_space (), true); (although if we go down that route, I hope we can add an exception to the formatting rule so that no space should be used before "()".) I suppose with the magic values it would be: if (rtx_mem mem = as_a (x[0])) *total = address_cost (mem[0], mem.mode (), mem.address_space (), true); but I'm not sure that that would really be more readable. FWIW I also did an experiment locally with replacing "rtx *" (rather than "rtx") with a "smart" pointer so that we could have four allocation areas: permanent, gty, function and temporary, with the pointer automatically promoting rtxes to the right allocation area for the containing object. Having a smart pointer made it suprisingly uninvasive but there was probably too much C++ magic involved in the handling of XEXP, etc., for the patch to be acceptable. Still, it did noticeably reduce max RSS and was a significant compile-time saving for extreme compiles like insn-recog.ii. Hope to return to it sometime... > Another thing is rtx construction in C++ code, which could look like: > > emit_insn (rtx_insn (rtx_set (rtx_reg (0), > rtx_plus (rtx_reg (1), rtx_reg (2))))); > emit_insn (rtx_barrier ()); > > Although I'm not sure how often this is needed in current practice, > since most of the time rtx instances are created from the .md patterns. > Maybe it could be useful for implementing some kind of ad-hoc rtx > matching, as it's found in cost calculations, predicates, constrants, > e.g. > > if ((GET_CODE (XEXP (x, 0)) == SMAX || GET_CODE (XEXP (x, 0)) == SMIN) > && CONST_INT_P (XEXP (XEXP (x, 0), 1)) > && REG_P (XEXP (XEXP (x, 0), 0)) > && CONST_INT_P (XEXP (x, 1))) > > could become: > if (matches (x, rtx_smax (reg_rtx (), const_int (), const_int ())) > || matches (x, rtx_smin (reg_rtx (), const_int (), const_int ())) > > somehow I find that easier to read and write. It sounds like this would be creating temporary rtxes though, which would be too expensive for matching. Maybe it would make sense to have a separate set of matching objects that only live for the duration of the statement. Then you could have matching objects like "range (min, max)" to match a range of integers. But like you say, I'm not sure whether it would really be a win in the end. I think what makes this example so hard to read is again that we refuse to put XEXP (x, 0) in a temporary variable and just write it out in full 4 times instead. if ((GET_CODE (op0) == SMAX || GET_CODE (op0) == SMIN) && CONST_INT_P (XEXP (op0, 1)) && REG_P (XEXP (op0, 0)) && CONST_INT_P (op1)) is a bit more obvious. Thanks, Richard