From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24056 invoked by alias); 7 Jun 2014 20:26:00 -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 24042 invoked by uid 89); 7 Jun 2014 20:25:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yh0-f51.google.com Received: from mail-yh0-f51.google.com (HELO mail-yh0-f51.google.com) (209.85.213.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sat, 07 Jun 2014 20:25:57 +0000 Received: by mail-yh0-f51.google.com with SMTP id f73so1516069yha.38 for ; Sat, 07 Jun 2014 13:25:54 -0700 (PDT) X-Received: by 10.236.140.233 with SMTP id e69mr1358744yhj.120.1402172754799; Sat, 07 Jun 2014 13:25:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.170.122.2 with HTTP; Sat, 7 Jun 2014 13:25:13 -0700 (PDT) In-Reply-To: <87vbscppva.fsf@talisman.default> References: <87vbscppva.fsf@talisman.default> From: Steven Bosscher Date: Sat, 07 Jun 2014 20:26:00 -0000 Message-ID: Subject: Re: RFA: Rework FOR_BB_INSNS iterators To: GCC Patches , "rdsandiford@googlemail.com" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00676.txt.bz2 On Sat, Jun 7, 2014 at 7:54 PM, Richard Sandiford wrote: > The two parts of the loop condition are really handling two different > kinds of block: ones like entry and exit that are completely empty > and normal ones that have at least a block note. There's no real > need to check for null INSNs in normal blocks. Block notes should go away some day, they're just remains of a time when there was no actual CFG in the compiler. > Also, refetching NEXT_INSN (BB_END (BB)) for each iteration can be expensive. > If we're prepared to say that the loop body can't insert instructions > for another block immediately after BB_END, This can happen with "block_label()" if e.g. a new jump is inserted for one reason or another. Not very likely for passes working in cfglayout mode, but post-RA there may be places that need this (splitters, peepholes, machine dependent reorgs, etc.). So even if we're prepared to say what you suggest, I don't think you can easily enforce it. > It's easier to change these macros if they define the INSN variables > themselves. If you're going to hack these iterators anyway (much appreciated BTW), I suggest to make them similar to the gsi, loop, edge, and bitmap iterators: A new "insn_iterator" structure to hold the variables and static inline functions wrapped in the macros. This will also be helpful if (when) we ever manage to make the type for an insn a non-rtx... > +/* For iterating over insns in a basic block. The iterator allows the loop > + body to delete INSN. It also ignores any instructions that the body > + inserts between INSN and the following instruction. */ > +#define FOR_BB_INSNS(BB, INSN) \ > + for (rtx INSN = BB_HEAD (BB), INSN##_cond_ = INSN, INSN##_next_, \ > + INSN##_end_ = INSN ? NEXT_INSN (BB_END (BB)) : NULL_RTX; \ > + INSN##_cond_ && (INSN##_next_ = NEXT_INSN (INSN), true); \ > + INSN = INSN##_next_, \ > + INSN##_cond_ = (INSN != INSN##_end_ ? (rtx) 1 : NULL_RTX)) This just makes my eyes hurt... What about cases where a FOR_BB_INSNS is terminated before reaching the end of a basic block, and you need to know at what insn you stopped? Up to now, you could do: rtx insn; basic_block bb; FOR_BB_INSNS (bb, insn) { ... // do stuff if (something) break; } do_something_with (insn); Looks like this is no longer possible with the implementation of FOR_BB_INSNS of your patch. I would not approve this patch, but let's wait what others think of it. Ciao! Steven