| Message ID | 87vbscppva.fsf@talisman.default |
|---|---|
| State | New |
| Headers | show |
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
Steven Bosscher <stevenb.gcc@gmail.com> writes: > 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. Yeah. I suppose when that happens empty blocks would look just like entry and exit as far as these iterators go. >> 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. Probably true. But if we want to allow insertions after BB_END, we need to make FOR_BB_INSNS_SAFE handle that for INSN == BB_END too. The alternative definition: for (rtx INSN = BB_HEAD (BB), INSN##_cond_ = INSN, INSN##_next_; \ INSN##_cond_ \ && (INSN##_next_ = NEXT_INSN (INSN), \ INSN##_cond_ = BB_END (BB), \ true); \ INSN##_cond_ = (INSN == INSN##_cond_ ? NULL_RTX : (rtx) 1), \ INSN = INSN##_next_) works too. It isn't quite as fast, but obviously correctness comes first. >> 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... Well, with this and... >> +/* 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. ...this I suppose it depends where we want to go with these iterators. I'm hoping eventually we'll move to C++11, where the natural way of writing the loop would be: for (rtx insn : bb->insns ()) (Or "auto *" instead of "rtx" if you prefer.) And I think the idiom of having the FOR_* macro define the iterator variable is much closer to that than: rtx insn; FOR_BB_INSNS (iter, bb, insn) would be. It's true that you can't leave "insn" with a signficant value after the loop, but no current code wants to do that. Personally I like the fact that loops that do want to set a final value have to make that explicit. When the vast majority (currently all) instances of: FOR_BB_INSNS (bb, insn) treat "insn" as local to the loop, it's helpful when the exceptions are obvious. I think if anything the patch would make it easier to change the type of insns to something other than rtx. It would just mean changing the "rtx" at the start of the two "for" loops to the new type, whereas at the moment you would need to change all the separate "rtx insn"s. (In particular, it's common for "insn" to be defined on the same line as other rtx variables that might need to stay as "rtx"es after the change.) Thanks, Richard
On Mon, 2014-06-09 at 20:32 +0100, Richard Sandiford wrote: > Steven Bosscher <stevenb.gcc@gmail.com> writes: > > 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. > > Yeah. I suppose when that happens empty blocks would look just like > entry and exit as far as these iterators go. > > >> 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. > > Probably true. But if we want to allow insertions after BB_END, > we need to make FOR_BB_INSNS_SAFE handle that for INSN == BB_END too. > > The alternative definition: > > for (rtx INSN = BB_HEAD (BB), INSN##_cond_ = INSN, INSN##_next_; \ > INSN##_cond_ \ > && (INSN##_next_ = NEXT_INSN (INSN), \ > INSN##_cond_ = BB_END (BB), \ > true); \ > INSN##_cond_ = (INSN == INSN##_cond_ ? NULL_RTX : (rtx) 1), \ > INSN = INSN##_next_) > > works too. It isn't quite as fast, but obviously correctness comes first. > > >> 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... > > Well, with this and... > > >> +/* 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. > > ...this I suppose it depends where we want to go with these iterators. > I'm hoping eventually we'll move to C++11, where the natural way of > writing the loop would be: > > for (rtx insn : bb->insns ()) > > (Or "auto *" instead of "rtx" if you prefer.) > > And I think the idiom of having the FOR_* macro define the iterator > variable is much closer to that than: > > rtx insn; > FOR_BB_INSNS (iter, bb, insn) > > would be. > > It's true that you can't leave "insn" with a signficant value after > the loop, but no current code wants to do that. Personally I like > the fact that loops that do want to set a final value have to make > that explicit. When the vast majority (currently all) instances of: > > FOR_BB_INSNS (bb, insn) > > treat "insn" as local to the loop, it's helpful when the exceptions > are obvious. > > I think if anything the patch would make it easier to change the > type of insns to something other than rtx. (sorry for the belated response; I was on vacation). FWIW I'm actually working on such a change, or, at least, to make instructions be a subclass of rtx_def; presumably this should make it easier to make it an entirely separate type, if that's desirable. Executive summary is that it's still a work in progress, and I'm going to be giving a talk about it at Cauldron next month ("A proposal for typesafe RTL", currently scheduled for the Sunday at 12.45 in Stream 2). More detailed version: I experimented with this a few months ago, and came up with a 159-patch patch series: http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v1/ That patch kit builds on x86_64 (and regrtests iirc), and uses a separate subclass for insns e.g. in the core basic block class, in the scheduler, register allocators, etc, but: (A) it only builds on about 70 of the ~200 configurations in config-list.mk (B) there was a tangled mess of dependencies between the patches making it hard to fix (A) (C) I went with the rather verbose "rtx_base_insn" name, which in v1 is a typedef for a pointer to an insn, along with a family of other typedefs, which I now realize is frowned upon e.g.: https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01632.html (D) I was using as_a *methods* rather than just as_a <>, like in v1 of my gimple-classes patches: (https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00887.html) So I've been reworking it, incorporating feedback from the gimple statement classes work; the latest patch series is v7 and can be seen here: http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v7/ It builds and regrtests on x86_64 on top of r211061 (aka dcd5393fd5b3ac53775a5546f3103958b64789bb) and builds on 184 of the configurations in config-list.mk; I have patches to fix the remaining ones to achieve parity with an unpatched build. The current class hierarchy looks like this: /* Subclasses of rtx_def, using indentation to show the class hierarchy, along with the relevant invariant. */ class rtx_def; class rtx_insn; /* (INSN_P (X) || NOTE_P (X) || JUMP_TABLE_DATA_P (X) || BARRIER_P (X) || LABEL_P (X)) */ class rtx_real_insn; /* INSN_P (X) */ class rtx_debug_insn; /* DEBUG_INSN_P (X) */ class rtx_nonjump_insn; /* NONJUMP_INSN_P (X) */ class rtx_jump_insn; /* JUMP_P (X) */ class rtx_call_insn; /* CALL_P (X) */ class rtx_jump_table_data; /* JUMP_TABLE_DATA_P (X) */ class rtx_barrier; /* BARRIER_P (X) */ class rtx_code_label; /* LABEL_P (X) */ class rtx_note; /* NOTE_P (X) */ and the code now uses "rtx_insn *" in hundreds of places where it would previously have used "rtx". My aim is that it always builds on every config: each patch is self-contained. To do this, the initial patches add scaffolding that adds some strategically-placed checked downcasts to rtx_insn * (e.g. on the result of PREV_INSN/NEXT_INSN). The subsequent patches then use these promises in order to strengthen the insn-handling code to work on the new subclass. The idea is that the final patches then remove this scaffolding, with the core BB types becoming rtx_insn *. The drawback of course is that during the process the resulting compiler is much slower - until the scaffolding is removed. v1 of the patch series also added some non-insn subclasses of rtx, for EXPR_LIST, INSN_LIST, and for SEQUENCE, allowing rewriting of iterations like this (in jump.c:rebuild_jump_labels_1), where "insn" and "forced_labels" are INSN_LIST and hence are "rtx_insn_list *": - for (insn = forced_labels; insn; insn = XEXP (insn, 1)) - if (LABEL_P (XEXP (insn, 0))) - LABEL_NUSES (XEXP (insn, 0))++; + for (insn = forced_labels; insn; insn = insn->next ()) + if (LABEL_P (insn->element ())) + LABEL_NUSES (insn->element ())++; Thoughts? Dave > It would just mean changing > the "rtx" at the start of the two "for" loops to the new type, > whereas at the moment you would need to change all the separate > "rtx insn"s. (In particular, it's common for "insn" to be defined > on the same line as other rtx variables that might need to stay > as "rtx"es after the change.)
On Mon, 2014-06-23 at 14:54 -0400, David Malcolm wrote: > FWIW I'm actually working on such a change, or, at least, to make > instructions be a subclass of rtx_def; presumably this should make it > easier to make it an entirely separate type, if that's desirable. > > Executive summary is that it's still a work in progress, and I'm going > to be giving a talk about it at Cauldron next month ("A proposal for > typesafe RTL", currently scheduled for the Sunday at 12.45 in Stream 2). > > More detailed version: > I experimented with this a few months ago, and came up with a 159-patch > patch series: > http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v1/ > > That patch kit builds on x86_64 (and regrtests iirc), and uses a > separate subclass for insns e.g. in the core basic block class, in the > scheduler, register allocators, etc, but: > (A) it only builds on about 70 of the ~200 configurations in > config-list.mk > (B) there was a tangled mess of dependencies between the patches > making it hard to fix (A) > (C) I went with the rather verbose "rtx_base_insn" name, which in v1 > is a typedef for a pointer to an insn, along with a family of other > typedefs, which I now realize is frowned upon e.g.: > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01632.html > (D) I was using as_a *methods* rather than just as_a <>, like > in v1 of my gimple-classes patches: > (https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00887.html) > > > So I've been reworking it, incorporating feedback from the gimple > statement classes work; the latest patch series is v7 and can be seen > here: > http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v7/ > > It builds and regrtests on x86_64 on top of r211061 (aka > dcd5393fd5b3ac53775a5546f3103958b64789bb) and builds on 184 of the > configurations in config-list.mk; I have patches to fix the remaining > ones to achieve parity with an unpatched build. > > The current class hierarchy looks like this: > /* Subclasses of rtx_def, using indentation to show the class > hierarchy, along with the relevant invariant. */ > class rtx_def; > class rtx_insn; /* (INSN_P (X) || NOTE_P (X) > || JUMP_TABLE_DATA_P (X) || BARRIER_P (X) > || LABEL_P (X)) */ > class rtx_real_insn; /* INSN_P (X) */ > class rtx_debug_insn; /* DEBUG_INSN_P (X) */ > class rtx_nonjump_insn; /* NONJUMP_INSN_P (X) */ > class rtx_jump_insn; /* JUMP_P (X) */ > class rtx_call_insn; /* CALL_P (X) */ > class rtx_jump_table_data; /* JUMP_TABLE_DATA_P (X) */ > class rtx_barrier; /* BARRIER_P (X) */ > class rtx_code_label; /* LABEL_P (X) */ > class rtx_note; /* NOTE_P (X) */ > > and the code now uses "rtx_insn *" in hundreds of places where it would > previously have used "rtx". > > My aim is that it always builds on every config: each patch is > self-contained. To do this, the initial patches add scaffolding that > adds some strategically-placed checked downcasts to rtx_insn * (e.g. on > the result of PREV_INSN/NEXT_INSN). The subsequent patches then use > these promises in order to strengthen the insn-handling code to work on > the new subclass. The idea is that the final patches then remove this > scaffolding, with the core BB types becoming rtx_insn *. The drawback > of course is that during the process the resulting compiler is much > slower - until the scaffolding is removed. > > v1 of the patch series also added some non-insn subclasses of rtx, for > EXPR_LIST, INSN_LIST, and for SEQUENCE, allowing rewriting of iterations > like this (in jump.c:rebuild_jump_labels_1), where "insn" and > "forced_labels" are INSN_LIST and hence are "rtx_insn_list *": > > - for (insn = forced_labels; insn; insn = XEXP (insn, 1)) > - if (LABEL_P (XEXP (insn, 0))) > - LABEL_NUSES (XEXP (insn, 0))++; > + for (insn = forced_labels; insn; insn = insn->next ()) > + if (LABEL_P (insn->element ())) > + LABEL_NUSES (insn->element ())++; > > Thoughts? 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. 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. Cheers, Oleg
David Malcolm <dmalcolm@redhat.com> writes: > On Mon, 2014-06-09 at 20:32 +0100, Richard Sandiford wrote: >> Steven Bosscher <stevenb.gcc@gmail.com> writes: >> > 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. >> >> Yeah. I suppose when that happens empty blocks would look just like >> entry and exit as far as these iterators go. >> >> >> 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. >> >> Probably true. But if we want to allow insertions after BB_END, >> we need to make FOR_BB_INSNS_SAFE handle that for INSN == BB_END too. >> >> The alternative definition: >> >> for (rtx INSN = BB_HEAD (BB), INSN##_cond_ = INSN, INSN##_next_; \ >> INSN##_cond_ \ >> && (INSN##_next_ = NEXT_INSN (INSN), \ >> INSN##_cond_ = BB_END (BB), \ >> true); \ >> INSN##_cond_ = (INSN == INSN##_cond_ ? NULL_RTX : (rtx) 1), \ >> INSN = INSN##_next_) >> >> works too. It isn't quite as fast, but obviously correctness comes first. >> >> >> 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... >> >> Well, with this and... >> >> >> +/* 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. >> >> ...this I suppose it depends where we want to go with these iterators. >> I'm hoping eventually we'll move to C++11, where the natural way of >> writing the loop would be: >> >> for (rtx insn : bb->insns ()) >> >> (Or "auto *" instead of "rtx" if you prefer.) >> >> And I think the idiom of having the FOR_* macro define the iterator >> variable is much closer to that than: >> >> rtx insn; >> FOR_BB_INSNS (iter, bb, insn) >> >> would be. >> >> It's true that you can't leave "insn" with a signficant value after >> the loop, but no current code wants to do that. Personally I like >> the fact that loops that do want to set a final value have to make >> that explicit. When the vast majority (currently all) instances of: >> >> FOR_BB_INSNS (bb, insn) >> >> treat "insn" as local to the loop, it's helpful when the exceptions >> are obvious. >> >> I think if anything the patch would make it easier to change the >> type of insns to something other than rtx. > > (sorry for the belated response; I was on vacation). > > FWIW I'm actually working on such a change, or, at least, to make > instructions be a subclass of rtx_def; presumably this should make it > easier to make it an entirely separate type, if that's desirable. > > Executive summary is that it's still a work in progress, and I'm going > to be giving a talk about it at Cauldron next month ("A proposal for > typesafe RTL", currently scheduled for the Sunday at 12.45 in Stream 2). > > More detailed version: > I experimented with this a few months ago, and came up with a 159-patch > patch series: > http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v1/ > > That patch kit builds on x86_64 (and regrtests iirc), and uses a > separate subclass for insns e.g. in the core basic block class, in the > scheduler, register allocators, etc, but: > (A) it only builds on about 70 of the ~200 configurations in > config-list.mk > (B) there was a tangled mess of dependencies between the patches > making it hard to fix (A) > (C) I went with the rather verbose "rtx_base_insn" name, which in v1 > is a typedef for a pointer to an insn, along with a family of other > typedefs, which I now realize is frowned upon e.g.: > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html > https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01632.html > (D) I was using as_a *methods* rather than just as_a <>, like > in v1 of my gimple-classes patches: > (https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00887.html) > > > So I've been reworking it, incorporating feedback from the gimple > statement classes work; the latest patch series is v7 and can be seen > here: > http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v7/ > > It builds and regrtests on x86_64 on top of r211061 (aka > dcd5393fd5b3ac53775a5546f3103958b64789bb) and builds on 184 of the > configurations in config-list.mk; I have patches to fix the remaining > ones to achieve parity with an unpatched build. > > The current class hierarchy looks like this: > /* Subclasses of rtx_def, using indentation to show the class > hierarchy, along with the relevant invariant. */ > class rtx_def; > class rtx_insn; /* (INSN_P (X) || NOTE_P (X) > || JUMP_TABLE_DATA_P (X) || BARRIER_P (X) > || LABEL_P (X)) */ > class rtx_real_insn; /* INSN_P (X) */ > class rtx_debug_insn; /* DEBUG_INSN_P (X) */ > class rtx_nonjump_insn; /* NONJUMP_INSN_P (X) */ > class rtx_jump_insn; /* JUMP_P (X) */ > class rtx_call_insn; /* CALL_P (X) */ > class rtx_jump_table_data; /* JUMP_TABLE_DATA_P (X) */ > class rtx_barrier; /* BARRIER_P (X) */ > class rtx_code_label; /* LABEL_P (X) */ > class rtx_note; /* NOTE_P (X) */ > > and the code now uses "rtx_insn *" in hundreds of places where it would > previously have used "rtx". This looks really good to me FWIW. The only thing I'm not sure about is how useful rtx_nonjump_insn would be. ISTM that jump_insn and call_insn really are subclasses that add extra information on top of a "normal" insn (JUMP_LABEL for jumps and CALL_INSN_FUNCTION_USAGE for calls). I suppose there's also a bikeshed argument about whether "rtx_" should be in the name, if we want to keep open the option of moving to a different structure. What do you think about adding something like: /* Account for the NEXT_INSN and BLOCK_FOR_INSN fields. */ rtunion GTY ((skip)) fld[2]; to rtx_insn, and similarly for the derived classes, so that each derived class has the right size? It might even help the compiler prove that it can speculatively fetch later parts of the structure before a condition based on something like BLOCK_FOR_INSN. > My aim is that it always builds on every config: each patch is > self-contained. To do this, the initial patches add scaffolding that > adds some strategically-placed checked downcasts to rtx_insn * (e.g. on > the result of PREV_INSN/NEXT_INSN). The subsequent patches then use > these promises in order to strengthen the insn-handling code to work on > the new subclass. The idea is that the final patches then remove this > scaffolding, with the core BB types becoming rtx_insn *. The drawback > of course is that during the process the resulting compiler is much > slower - until the scaffolding is removed. Do you get to the point where things like NEXT_INSN and PREV_INSN can require rtx_insns as argument? I suppose that would be the end goal. Same for JUMP_LABEL requiring an rtx_jump_insn, etc. > v1 of the patch series also added some non-insn subclasses of rtx, for > EXPR_LIST, INSN_LIST, and for SEQUENCE, allowing rewriting of iterations > like this (in jump.c:rebuild_jump_labels_1), where "insn" and > "forced_labels" are INSN_LIST and hence are "rtx_insn_list *": > > - for (insn = forced_labels; insn; insn = XEXP (insn, 1)) > - if (LABEL_P (XEXP (insn, 0))) > - LABEL_NUSES (XEXP (insn, 0))++; > + for (insn = forced_labels; insn; insn = insn->next ()) > + if (LABEL_P (insn->element ())) > + LABEL_NUSES (insn->element ())++; SEQUENCE is just weird though :-) It would be good to have an alternative representation, but that'd be a lot of work on reorg. Definitely agree that EXPR_LIST, INSN_LIST and INT_LIST are another natural and useful subclass. Thanks, Richard
Oleg Endo <oleg.endo@t-online.de> 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 <rtx_mem *> (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 <rtx_mem *> (...)) *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 <rtx_mem> (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
On 06/25/14 03:36, Richard Sandiford wrote: > > 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: Historical coding style :( It's particularly bad in RTL land, but you see the same problems in places like fold-const. > > if (rtx_mem *mem = as_a <rtx_mem *> (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 <rtx_mem *> (...)) > *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 "()".) There was some talk of revamping the rules of parens for member function calls. I don't recall what the final outcome was. I prefer the latter, but I tend to worry about making David's patches even more invasive than they already are :-) > > I suppose with the magic values it would be: > > if (rtx_mem mem = as_a <rtx_mem> (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. Please, no... > > 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. And probably faster as well since we've CSE'd the memory reference. In this specific example it probably doesn't matter, but often there's a call somewhere between XEXPs that we'd like to CSE that destroys our ability to CSE away memory references. This kind of problem is prevasive in the RTL analysis/optimizers.
On 06/25/14 02:54, Richard Sandiford wrote: >> >> and the code now uses "rtx_insn *" in hundreds of places where it would >> previously have used "rtx". > > This looks really good to me FWIW. The only thing I'm not sure about is > how useful rtx_nonjump_insn would be. ISTM that jump_insn and call_insn > really are subclasses that add extra information on top of a "normal" > insn (JUMP_LABEL for jumps and CALL_INSN_FUNCTION_USAGE for calls). Glad you think so -- I hope others chime in as the work progresses so they're not saying WTF, why did all this change when I approve the massive patchkit. > > I suppose there's also a bikeshed argument about whether "rtx_" > should be in the name, if we want to keep open the option of > moving to a different structure. Well, as you're probably aware, I hate the "everything is an RTX" model we've got in GCC. I'm not aware of anything having the toplevel objects in the insn chain buys us other than bugs. > > What do you think about adding something like: > > /* Account for the NEXT_INSN and BLOCK_FOR_INSN fields. */ > rtunion GTY ((skip)) fld[2]; > > to rtx_insn, and similarly for the derived classes, so that each derived > class has the right size? It might even help the compiler prove that it > can speculatively fetch later parts of the structure before a condition > based on something like BLOCK_FOR_INSN. For objects which form the insn chain, I'd like the prev, next which each object must have would be in the base object. > Do you get to the point where things like NEXT_INSN and PREV_INSN can > require rtx_insns as argument? I suppose that would be the end goal. > Same for JUMP_LABEL requiring an rtx_jump_insn, etc. Yea, I think that's the ultimate goal of this stage. In my mind I wanted to narrow the scope enough that it had a chance to get into the upcoming release -- thus I've asked David to focus on the objects that show up in the insn chain. There may be some bleed out, but that's really the focus and if we could get to NEXT_INSN/PREV_INSN requiring an rtx_insn argument, then we'd be golden. > > SEQUENCE is just weird though :-) It would be good to have an alternative > representation, but that'd be a lot of work on reorg. Yea. And I don't think anyone is keen on rewriting reorg. Jeff
On Wed, Jun 25, 2014 at 10:46 PM, Jeff Law wrote: > On 06/25/14 02:54, Richard Sandiford wrote: >> >> SEQUENCE is just weird though :-) It would be good to have an alternative >> representation, but that'd be a lot of work on reorg. > > Yea. And I don't think anyone is keen on rewriting reorg. Rewriting/revamping reorg is not really the problem, IMHO. Last year I hacked a bit on a new delayed-branch scheduler, and I got results that were not too bad (especially considering my GCC time is only a few hours per week). The the real problem is designing that alternative representation of delay slots, and teaching the back ends about that. Just communicating a delayed-branch sequence to the back ends is pretty awful (a global variable in final.c) and a lot of back-end code has non-obvious assumptions about jumping across insns contained in SEQUENCEs. There's also one back end (mep?) that uses SEQUENCE for bundles (RTL abuse is not considered bad practice by all maintainers ;-). (I actually found SEQUENCE to be quite nice to work with when I allowed them to be the last insn in a basic block. One of my goals was to retain the CFG across dbr_sched, but that turned out to be blocked by other things than dbr_sched, like fake insns that some back ends emit between basic blocks, e.g. for constant pools). Having some kind of "insns container" like SEQUENCE would IMHO not be a bad thing, perhaps a necessity, and perhaps even an improvement (like for representing bundles), as long as we can assign sane semantics to it w.r.t. next/prev insn. SEQUENCE wasn't designed with its current application in mind... Ciao! Steven
On Wed, 2014-06-25 at 10:36 +0100, Richard Sandiford wrote: > Oleg Endo <oleg.endo@t-online.de> 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 <rtx_mem *> (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 <rtx_mem *> (...)) > *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 "()".) If such an exception is introduced, I can imagine it'd be difficult to judge on a case by case whether to apply the exceptional rule or not, since those are just function calls. But in the end it doesn't matter. It's just a matter of habbit :) > > I suppose with the magic values it would be: > > if (rtx_mem mem = as_a <rtx_mem> (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. No, essentially it's the same. Whether the current XEXP (x, n) is done by operator [] or a member function 'xexp (n)' doesn't matter. The biggest (optical) change is for nested XEXP... XEXP (XEXP (XEXP (x, a), b), c) becomes x.exp (a).exp (b).exp (c) > 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... Yep, passing the new rtx classes by value instead by pointer opens other doors, such as implementing smart pointers internally and what not. > > > 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. Yeah, as I wrote it up there, probably. It depends on the implementation of those default constructed objects. > 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. Maybe something like this ... static my_match_rtx = rtx_smax (reg_rtx (), const_int (), const_int ()); if (matches (x, my_match_rtx) ... > 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. In this case yes. On the other hand, that if statement is part of a bigger function. There are cases, where manual CSE is not so practical, e.g. when it's an if - else if - else if - else. Cheers, Oleg
On Wed, 2014-06-25 at 14:39 -0600, Jeff Law wrote: > On 06/25/14 03:36, Richard Sandiford wrote: > > > > 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: > Historical coding style :( It's particularly bad in RTL land, but you > see the same problems in places like fold-const. > > > > > > if (rtx_mem *mem = as_a <rtx_mem *> (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 <rtx_mem *> (...)) > > *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 "()".) > There was some talk of revamping the rules of parens for member function > calls. I don't recall what the final outcome was. > > I prefer the latter, but I tend to worry about making David's patches > even more invasive than they already are :-) :) Yeah, that's probably my primary concern here. The patch kit is going to be big (currently at 133 patches [1]), and so I want something that we can sanely keep track of, that is easily reviewable, and will be as easy as possible to merge. i.e. I don't want to get bogged down in a big revamp of the rest of the RTL interface if I can help it. I'm attempting to focus on splitting out insns from expressions. As mentioned before, in the v1 of this patch kit I also introduced rtx subclasses for various core types like INSN_LIST, SEQUENCE, etc - but in this iteration I'm attempting to do it without those - not yet sure if it's possible. If it's desirable to actually make insns be a separate class, I'm considering the goal of making the attributes of insns become actual fields, something like: class rtx_insn /* we can bikeshed over the name */ { public: rtx_insn *m_prev; rtx_insn *m_next; int m_uid; }; #define PREV_INSN(INSN) ((INSN)->m_prev) #define NEXT_INSN(INSN) ((INSN)->m_next) #define INSN_UID(INSN) ((INSN)->m_uid) /* or we could convert them to functions returning references, I guess */ ...etc for the subclasses, giving something that gdb can print sanely, and, I hope, more amenable to optimization when gcc itself is compiled. But even if we don't get there and simply keep insns as subclasses of rtx, I think that having insn-handling code marked as such in the type-system is a win from a readability standpoint. Either way, I think this should be much more "grokkable" by new contributors. My own experience is that RTL was the aspect of GCC I had most difficulty with when I was a newbie - hence my motivation to "drain this swamp". Hope these ideas sound sane Dave [1] FWIW the latest version of the patches is here: http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v9/ at 133 patches (as before it's relative to r211061). I'm aiming for lots of *small* patches. Successful bootstrap and regrtest on x86_64; builds on 187 configurations. Perhaps the biggest change since last time is that the "scaffolding" phase of the patches introduces a hack into the generated inns-*.c so that the .md files see rtx_insn * rather than plain rtx (for "insn" and "curr_insn"), which should allow lots of target-specific hooks used from .md files to be similarly converted: http://dmalcolm.fedorapeople.org/gcc/patch-backups/rtx-classes/v9/0036-Use-rtx_insn-internally-within-generated-functions.patch > > I suppose with the magic values it would be: > > > > if (rtx_mem mem = as_a <rtx_mem> (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. > Please, no... > > > > > > 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. > And probably faster as well since we've CSE'd the memory reference. In > this specific example it probably doesn't matter, but often there's a > call somewhere between XEXPs that we'd like to CSE that destroys our > ability to CSE away memory references. > > This kind of problem is prevasive in the RTL analysis/optimizers. >
On Wed, 2014-06-25 at 10:36 +0100, Richard Sandiford wrote: > Oleg Endo <oleg.endo@t-online.de> 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 <rtx_mem *> (XEXP (x, 0))) > *total = address_cost (XEXP (mem, 0), GET_MODE (mem), > MEM_ADDR_SPACE (mem), true); FWIW you want a dyn_cast<> rather than an as_a<> here, giving: if (rtx_mem *mem = dyn_cast <rtx_mem *> (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 <rtx_mem *> (...)) > *total = address_cost (mem->address (), mem->mode (), mem->address_space (), > true); (likewise) > (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 <rtx_mem> (x[0])) > *total = address_cost (mem[0], mem.mode (), mem.address_space (), true); (likewise). > but I'm not sure that that would really be more readable. [...snip...; see my other mail for notes on restricting the scope of the current patch kit to an insn vs expr separation, for the sake of my sanity :) ]
On 06/27/14 08:26, David Malcolm wrote: > > Yeah, that's probably my primary concern here. The patch kit is going > to be big (currently at 133 patches [1]), and so I want something that > we can sanely keep track of, that is easily reviewable, and will be as > easy as possible to merge. > > i.e. I don't want to get bogged down in a big revamp of the rest of the > RTL interface if I can help it. Precisely. After revamping the objects at the toplevel of the insn chain, we can evaluate what project makes the most sense to tackle. > > If it's desirable to actually make insns be a separate class, I'm > considering the goal of making the attributes of insns become actual > fields, something like: I think having the toplevel objects in the insn chain as a separate class makes sense. My biggest concerns are a variety of implementation details like is there code that wants to use the various rtl walkers on those toplevel objects. Which (and I hate to say it) makes me wonder if this is a two step process. First step is to have the subclass style implementation. Then we look deeper at what would need to change to break those toplevel objects out into a distinct class. In theory if we do things right, we leverage the new types and static checking to catch all the "don't assume the toplevel objects in the insn chain are rtxs" issues. Two stage also gives others a chance to chime in if they're aware of good reasons not to make the change. > > But even if we don't get there and simply keep insns as subclasses of > rtx, I think that having insn-handling code marked as such in the > type-system is a win from a readability standpoint. Absolutely. > > Hope these ideas sound sane They do. I think we're very much on the same page here. Jeff
Index: gcc/basic-block.h =================================================================== --- gcc/basic-block.h 2014-06-07 18:23:27.319167704 +0100 +++ gcc/basic-block.h 2014-06-07 18:51:42.821002739 +0100 @@ -336,28 +336,23 @@ #define FOR_EACH_BB_FN(BB, FN) \ #define FOR_EACH_BB_REVERSE_FN(BB, FN) \ FOR_BB_BETWEEN (BB, (FN)->cfg->x_exit_block_ptr->prev_bb, (FN)->cfg->x_entry_block_ptr, prev_bb) -/* For iterating over insns in basic block. */ -#define FOR_BB_INSNS(BB, INSN) \ - for ((INSN) = BB_HEAD (BB); \ - (INSN) && (INSN) != NEXT_INSN (BB_END (BB)); \ - (INSN) = NEXT_INSN (INSN)) - -/* For iterating over insns in basic block when we might remove the - current insn. */ -#define FOR_BB_INSNS_SAFE(BB, INSN, CURR) \ - for ((INSN) = BB_HEAD (BB), (CURR) = (INSN) ? NEXT_INSN ((INSN)): NULL; \ - (INSN) && (INSN) != NEXT_INSN (BB_END (BB)); \ - (INSN) = (CURR), (CURR) = (INSN) ? NEXT_INSN ((INSN)) : NULL) +/* 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)) +/* Likewise, but in reverse. */ #define FOR_BB_INSNS_REVERSE(BB, INSN) \ - for ((INSN) = BB_END (BB); \ - (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB)); \ - (INSN) = PREV_INSN (INSN)) - -#define FOR_BB_INSNS_REVERSE_SAFE(BB, INSN, CURR) \ - for ((INSN) = BB_END (BB),(CURR) = (INSN) ? PREV_INSN ((INSN)) : NULL; \ - (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB)); \ - (INSN) = (CURR), (CURR) = (INSN) ? PREV_INSN ((INSN)) : NULL) + for (rtx INSN = BB_END (BB), INSN##_cond_ = INSN, INSN##_prev_, \ + INSN##_end_ = INSN ? PREV_INSN (BB_HEAD (BB)) : NULL_RTX; \ + INSN##_cond_ && (INSN##_prev_ = PREV_INSN (INSN), true); \ + INSN = INSN##_prev_, \ + INSN##_cond_ = (INSN != INSN##_end_ ? (rtx) 1 : NULL_RTX)) /* Cycles through _all_ basic blocks, even the fake ones (entry and exit block). */ Index: gcc/alias.c =================================================================== --- gcc/alias.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/alias.c 2014-06-07 18:51:38.344967876 +0100 @@ -2840,7 +2840,7 @@ init_alias_analysis (void) int changed, pass; int i; unsigned int ui; - rtx insn, val; + rtx val; int rpo_cnt; int *rpo; Index: gcc/bb-reorder.c =================================================================== --- gcc/bb-reorder.c 2014-06-07 18:23:27.320167705 +0100 +++ gcc/bb-reorder.c 2014-06-07 18:51:38.345967884 +0100 @@ -1331,7 +1331,6 @@ copy_bb_p (const_basic_block bb, int cod { int size = 0; int max_size = uncond_jump_length; - rtx insn; if (!bb->frequency) return false; @@ -2443,7 +2442,6 @@ pass_duplicate_computed_gotos::execute ( mark it in the candidates. */ FOR_EACH_BB_FN (bb, fun) { - rtx insn; edge e; edge_iterator ei; int size, all_flags; Index: gcc/cfgloop.c =================================================================== --- gcc/cfgloop.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/cfgloop.c 2014-06-07 18:51:38.407968367 +0100 @@ -1736,7 +1736,6 @@ loop_exits_from_bb_p (struct loop *loop, location_t get_loop_location (struct loop *loop) { - rtx insn = NULL; struct niter_desc *desc = NULL; edge exit; Index: gcc/cfgloopanal.c =================================================================== --- gcc/cfgloopanal.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/cfgloopanal.c 2014-06-07 18:51:38.345967884 +0100 @@ -174,7 +174,6 @@ num_loop_insns (const struct loop *loop) { basic_block *bbs, bb; unsigned i, ninsns = 0; - rtx insn; bbs = get_loop_body (loop); for (i = 0; i < loop->num_nodes; i++) @@ -198,7 +197,6 @@ average_num_loop_insns (const struct loo { basic_block *bbs, bb; unsigned i, binsns, ninsns, ratio; - rtx insn; ninsns = 0; bbs = get_loop_body (loop); Index: gcc/cfgrtl.c =================================================================== --- gcc/cfgrtl.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/cfgrtl.c 2014-06-07 18:51:38.347967899 +0100 @@ -2646,8 +2646,6 @@ rtl_verify_bb_pointers (void) /* Check the general integrity of the basic blocks. */ FOR_EACH_BB_REVERSE_FN (bb, cfun) { - rtx insn; - if (!(bb->flags & BB_RTL)) { error ("BB_RTL flag not set for block %d", bb->index); @@ -2664,7 +2662,7 @@ rtl_verify_bb_pointers (void) err = 1; } - for (insn = BB_HEADER (bb); insn; insn = NEXT_INSN (insn)) + for (rtx insn = BB_HEADER (bb); insn; insn = NEXT_INSN (insn)) if (!BARRIER_P (insn) && BLOCK_FOR_INSN (insn) != NULL) { @@ -2672,7 +2670,7 @@ rtl_verify_bb_pointers (void) INSN_UID (insn), bb->index); err = 1; } - for (insn = BB_FOOTER (bb); insn; insn = NEXT_INSN (insn)) + for (rtx insn = BB_FOOTER (bb); insn; insn = NEXT_INSN (insn)) if (!BARRIER_P (insn) && BLOCK_FOR_INSN (insn) != NULL) { @@ -4671,8 +4669,6 @@ rtl_make_forwarder_block (edge fallthru static bool rtl_block_empty_p (basic_block bb) { - rtx insn; - if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun) || bb == EXIT_BLOCK_PTR_FOR_FN (cfun)) return true; @@ -4690,7 +4686,6 @@ rtl_block_empty_p (basic_block bb) static basic_block rtl_split_block_before_cond_jump (basic_block bb) { - rtx insn; rtx split_point = NULL; rtx last = NULL; bool found_code = false; @@ -4999,7 +4994,6 @@ rtl_duplicate_bb (basic_block bb) rtl_account_profile_record (basic_block bb, int after_pass, struct profile_record *record) { - rtx insn; FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) { Index: gcc/combine.c =================================================================== --- gcc/combine.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/combine.c 2014-06-07 18:51:38.351967931 +0100 @@ -984,7 +984,7 @@ delete_noop_moves (void) create_log_links (void) { basic_block bb; - rtx *next_use, insn; + rtx *next_use; df_ref *def_vec, *use_vec; next_use = XCNEWVEC (rtx, max_reg_num ()); @@ -1108,7 +1108,7 @@ insn_a_feeds_b (rtx a, rtx b) static int combine_instructions (rtx f, unsigned int nregs) { - rtx insn, next; + rtx next; #ifdef HAVE_cc0 rtx prev; #endif @@ -1226,7 +1226,7 @@ combine_instructions (rtx f, unsigned in last_bb = this_basic_block; rtl_profile_for_bb (this_basic_block); - for (insn = BB_HEAD (this_basic_block); + for (rtx insn = BB_HEAD (this_basic_block); insn != NEXT_INSN (BB_END (this_basic_block)); insn = next ? next : NEXT_INSN (insn)) { Index: gcc/cprop.c =================================================================== --- gcc/cprop.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/cprop.c 2014-06-07 18:51:38.376968126 +0100 @@ -402,8 +402,6 @@ compute_hash_table_work (struct hash_tab FOR_EACH_BB_FN (bb, cfun) { - rtx insn; - /* Reset tables used to keep track of what's not yet invalid [since the end of the block]. */ CLEAR_REG_SET (reg_set_bitmap); @@ -1229,7 +1227,6 @@ do_local_cprop (rtx x, rtx insn) local_cprop_pass (void) { basic_block bb; - rtx insn; bool changed = false; unsigned i; @@ -1660,7 +1657,6 @@ bypass_conditional_jumps (void) basic_block bb; int changed; rtx setcc; - rtx insn; rtx dest; /* Note we start at block 1. */ @@ -1825,7 +1821,6 @@ one_cprop_pass (void) if (set_hash_table.n_elems > 0) { basic_block bb; - rtx insn; alloc_cprop_mem (last_basic_block_for_fn (cfun), set_hash_table.n_elems); Index: gcc/cse.c =================================================================== --- gcc/cse.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/cse.c 2014-06-07 18:51:38.378968141 +0100 @@ -6359,7 +6359,6 @@ cse_prescan_path (struct cse_basic_block for (path_entry = 0; path_entry < path_size; path_entry++) { basic_block bb; - rtx insn; bb = data->path[path_entry].bb; @@ -6398,7 +6397,6 @@ cse_extended_basic_block (struct cse_bas for (path_entry = 0; path_entry < path_size; path_entry++) { basic_block bb; - rtx insn; bb = ebb_data->path[path_entry].bb; @@ -6522,7 +6520,7 @@ cse_extended_basic_block (struct cse_bas /* If this is a conditional jump insn, record any known equivalences due to the condition being tested. */ - insn = BB_END (bb); + rtx insn = BB_END (bb); if (path_entry < path_size - 1 && JUMP_P (insn) && single_set (insn) Index: gcc/df-core.c =================================================================== --- gcc/df-core.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/df-core.c 2014-06-07 18:51:38.378968141 +0100 @@ -1946,7 +1946,6 @@ df_set_clean_cfg (void) df_ref df_bb_regno_first_def_find (basic_block bb, unsigned int regno) { - rtx insn; df_ref *def_rec; unsigned int uid; @@ -1972,7 +1971,6 @@ df_bb_regno_first_def_find (basic_block df_ref df_bb_regno_last_def_find (basic_block bb, unsigned int regno) { - rtx insn; df_ref *def_rec; unsigned int uid; Index: gcc/df-problems.c =================================================================== --- gcc/df-problems.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/df-problems.c 2014-06-07 18:51:38.380968157 +0100 @@ -355,7 +355,6 @@ df_rd_bb_local_compute (unsigned int bb_ { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_rd_bb_info *bb_info = df_rd_get_bb_info (bb_index); - rtx insn; bitmap_clear (&seen_in_block); bitmap_clear (&seen_in_insn); @@ -835,7 +834,6 @@ df_lr_bb_local_compute (unsigned int bb_ { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_lr_bb_info *bb_info = df_lr_get_bb_info (bb_index); - rtx insn; df_ref *def_rec; df_ref *use_rec; @@ -1462,7 +1460,6 @@ df_live_bb_local_compute (unsigned int b { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_live_bb_info *bb_info = df_live_get_bb_info (bb_index); - rtx insn; df_ref *def_rec; int luid = 0; @@ -1982,7 +1979,6 @@ df_chain_remove_problem (void) EXECUTE_IF_SET_IN_BITMAP (df_chain->out_of_date_transfer_functions, 0, bb_index, bi) { - rtx insn; df_ref *def_rec; df_ref *use_rec; basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); @@ -2105,7 +2101,6 @@ df_chain_create_bb (unsigned int bb_inde { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_rd_bb_info *bb_info = df_rd_get_bb_info (bb_index); - rtx insn; bitmap_head cpy; bitmap_initialize (&cpy, &bitmap_default_obstack); @@ -2531,7 +2526,6 @@ df_word_lr_bb_local_compute (unsigned in { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_word_lr_bb_info *bb_info = df_word_lr_get_bb_info (bb_index); - rtx insn; df_ref *def_rec; df_ref *use_rec; @@ -3153,7 +3147,6 @@ df_note_bb_compute (unsigned int bb_inde bitmap live, bitmap do_not_gen, bitmap artificial_uses) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; df_ref *def_rec; df_ref *use_rec; struct dead_debug_local debug; @@ -4271,7 +4264,6 @@ df_md_bb_local_compute (unsigned int bb_ { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); struct df_md_bb_info *bb_info = df_md_get_bb_info (bb_index); - rtx insn; /* Artificials are only hard regs. */ if (!(df->changeable_flags & DF_NO_HARD_REGS)) Index: gcc/df-scan.c =================================================================== --- gcc/df-scan.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/df-scan.c 2014-06-07 18:51:38.381968164 +0100 @@ -283,7 +283,6 @@ df_scan_free_bb_info (basic_block bb, vo /* See if bb_info is initialized. */ if (bb_info->artificial_defs) { - rtx insn; FOR_BB_INSNS (bb, insn) { if (INSN_P (insn)) @@ -403,7 +402,6 @@ df_scan_start_dump (FILE *file ATTRIBUTE int icount = 0; int ccount = 0; basic_block bb; - rtx insn; fprintf (file, ";; invalidated by call \t"); df_print_regset (file, regs_invalidated_by_call_regset); @@ -482,7 +480,6 @@ df_scan_start_block (basic_block bb, FIL } #if 0 { - rtx insn; FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) df_insn_debug (insn, false, file); @@ -1416,13 +1413,8 @@ df_insn_rescan_all (void) bitmap_clear (&df->insns_to_notes_rescan); FOR_EACH_BB_FN (bb, cfun) - { - rtx insn; - FOR_BB_INSNS (bb, insn) - { - df_insn_rescan (insn); - } - } + FOR_BB_INSNS (bb, insn) + df_insn_rescan (insn); if (no_insn_rescan) df_set_flags (DF_NO_INSN_RESCAN); @@ -1638,7 +1630,6 @@ df_reorganize_refs_by_reg_by_insn (struc EXECUTE_IF_SET_IN_BITMAP (df->blocks_to_analyze, 0, bb_index, bi) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; df_ref *ref_rec; if (include_defs) @@ -1692,7 +1683,6 @@ df_reorganize_refs_by_reg_by_insn (struc EXECUTE_IF_SET_IN_BITMAP (df->blocks_to_analyze, 0, bb_index, bi) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; df_ref *ref_rec; if (include_defs) @@ -1827,8 +1817,6 @@ df_reorganize_refs_by_insn_bb (basic_blo bool include_defs, bool include_uses, bool include_eq_uses) { - rtx insn; - if (include_defs) offset = df_add_refs_to_table (offset, ref_info, df_get_artificial_defs (bb->index)); @@ -3528,7 +3516,6 @@ df_insn_refs_collect (struct df_collecti void df_recompute_luids (basic_block bb) { - rtx insn; int luid = 0; df_grow_insn_info (); @@ -3622,7 +3609,6 @@ df_bb_refs_collect (struct df_collection df_bb_refs_record (int bb_index, bool scan_insns) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; int luid = 0; if (!df) @@ -4158,14 +4144,9 @@ df_update_entry_exit_and_calls (void) /* The call insns need to be rescanned because there may be changes in the set of registers clobbered across the call. */ FOR_EACH_BB_FN (bb, cfun) - { - rtx insn; - FOR_BB_INSNS (bb, insn) - { - if (INSN_P (insn) && CALL_P (insn)) - df_insn_rescan (insn); - } - } + FOR_BB_INSNS (bb, insn) + if (INSN_P (insn) && CALL_P (insn)) + df_insn_rescan (insn); } @@ -4430,7 +4411,6 @@ df_insn_refs_verify (struct df_collectio static bool df_bb_verify (basic_block bb) { - rtx insn; struct df_scan_bb_info *bb_info = df_scan_get_bb_info (bb->index); struct df_collection_rec collection_rec; Index: gcc/dse.c =================================================================== --- gcc/dse.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/dse.c 2014-06-07 18:51:38.382968172 +0100 @@ -2710,7 +2710,6 @@ dse_step1 (void) if (bb->index >= NUM_FIXED_BLOCKS) { - rtx insn; cse_store_info_pool = create_alloc_pool ("cse_store_info_pool", Index: gcc/function.c =================================================================== --- gcc/function.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/function.c 2014-06-07 18:51:38.383968180 +0100 @@ -5979,7 +5979,7 @@ reposition_prologue_and_epilogue_notes ( FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) { - rtx insn, first = NULL, note = NULL; + rtx first = NULL, note = NULL; basic_block bb = e->src; /* Scan from the beginning until we reach the first epilogue insn. */ @@ -6444,7 +6444,7 @@ const pass_data pass_data_match_asm_cons pass_match_asm_constraints::execute (function *fun) { basic_block bb; - rtx insn, pat, *p_sets; + rtx pat, *p_sets; int noutputs; if (!crtl->has_asm_statement) Index: gcc/fwprop.c =================================================================== --- gcc/fwprop.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/fwprop.c 2014-06-07 18:51:38.384968188 +0100 @@ -220,7 +220,6 @@ single_def_use_dom_walker::before_dom_ch int bb_index = bb->index; struct df_md_bb_info *md_bb_info = df_md_get_bb_info (bb_index); struct df_lr_bb_info *lr_bb_info = df_lr_get_bb_info (bb_index); - rtx insn; bitmap_copy (local_md, &md_bb_info->in); bitmap_copy (local_lr, &lr_bb_info->in); Index: gcc/gcse.c =================================================================== --- gcc/gcse.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/gcse.c 2014-06-07 18:51:38.385968195 +0100 @@ -1561,7 +1561,6 @@ compute_hash_table_work (struct hash_tab FOR_EACH_BB_FN (current_bb, cfun) { - rtx insn; unsigned int regno; /* First pass over the instructions records information used to @@ -3234,7 +3233,6 @@ hoist_code (void) FOR_EACH_BB_FN (bb, cfun) { - rtx insn; int to_head; to_head = 0; @@ -3564,7 +3562,6 @@ calculate_bb_reg_pressure (void) { int i; unsigned int j; - rtx insn; basic_block bb; bitmap curr_regs_live; bitmap_iterator bi; @@ -3943,7 +3940,6 @@ compute_ld_motion_mems (void) { struct ls_expr * ptr; basic_block bb; - rtx insn; pre_ldst_mems = NULL; pre_ldst_table.create (13); Index: gcc/ifcvt.c =================================================================== --- gcc/ifcvt.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ifcvt.c 2014-06-07 18:51:38.388968219 +0100 @@ -2429,8 +2429,6 @@ noce_can_store_speculate_p (basic_block dominator != NULL; dominator = get_immediate_dominator (CDI_POST_DOMINATORS, dominator)) { - rtx insn; - FOR_BB_INSNS (dominator, insn) { /* If we see something that might be a memory barrier, we @@ -2814,7 +2812,7 @@ cond_move_convert_if_block (struct noce_ bool else_block_p) { enum rtx_code code; - rtx insn, cond_arg0, cond_arg1; + rtx cond_arg0, cond_arg1; code = GET_CODE (cond); cond_arg0 = XEXP (cond, 0); @@ -4213,7 +4211,7 @@ dead_or_predicable (basic_block test_bb, /* Try the NCE path if the CE path did not result in any changes. */ if (n_validated_changes == 0) { - rtx cond, insn; + rtx cond; regset live; bool success; Index: gcc/init-regs.c =================================================================== --- gcc/init-regs.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/init-regs.c 2014-06-07 18:51:38.388968219 +0100 @@ -61,7 +61,6 @@ initialize_uninitialized_regs (void) FOR_EACH_BB_FN (bb, cfun) { - rtx insn; bitmap lr = DF_LR_IN (bb); bitmap ur = DF_LIVE_IN (bb); bitmap_clear (already_genned); Index: gcc/ira-build.c =================================================================== --- gcc/ira-build.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira-build.c 2014-06-07 18:51:38.413968413 +0100 @@ -1927,7 +1927,6 @@ create_insn_allocnos (rtx x, bool output create_bb_allocnos (ira_loop_tree_node_t bb_node) { basic_block bb; - rtx insn; unsigned int i; bitmap_iterator bi; Index: gcc/ira-conflicts.c =================================================================== --- gcc/ira-conflicts.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira-conflicts.c 2014-06-07 18:51:38.389968227 +0100 @@ -421,7 +421,6 @@ add_insn_allocno_copies (rtx insn) add_copies (ira_loop_tree_node_t loop_tree_node) { basic_block bb; - rtx insn; bb = loop_tree_node->bb; if (bb == NULL) Index: gcc/ira-costs.c =================================================================== --- gcc/ira-costs.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira-costs.c 2014-06-07 18:51:38.389968227 +0100 @@ -1495,8 +1495,6 @@ print_pseudo_costs (FILE *f) static void process_bb_for_costs (basic_block bb) { - rtx insn; - frequency = REG_FREQ_FROM_BB (bb); if (frequency == 0) frequency = 1; @@ -1888,7 +1886,7 @@ process_bb_node_for_hard_reg_moves (ira_ ira_loop_tree_node_t curr_loop_tree_node; enum reg_class rclass; basic_block bb; - rtx insn, set, src, dst; + rtx set, src, dst; bb = loop_tree_node->bb; if (bb == NULL) Index: gcc/ira-emit.c =================================================================== --- gcc/ira-emit.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira-emit.c 2014-06-07 18:51:38.390968234 +0100 @@ -557,7 +557,7 @@ change_loop (ira_loop_tree_node_t node) int regno; bool used_p; ira_allocno_t allocno, parent_allocno, *map; - rtx insn, original_reg; + rtx original_reg; enum reg_class aclass, pclass; ira_loop_tree_node_t parent; @@ -1234,7 +1234,6 @@ add_ranges_and_copies (void) ira_emit (bool loops_p) { basic_block bb; - rtx insn; edge_iterator ei; edge e; ira_allocno_t a; Index: gcc/ira-lives.c =================================================================== --- gcc/ira-lives.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira-lives.c 2014-06-07 18:51:38.414968422 +0100 @@ -1052,7 +1052,6 @@ process_bb_node_lives (ira_loop_tree_nod int i, freq; unsigned int j; basic_block bb; - rtx insn; bitmap_iterator bi; bitmap reg_live_out; unsigned int px; Index: gcc/ira.c =================================================================== --- gcc/ira.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ira.c 2014-06-07 18:51:38.391968242 +0100 @@ -2014,7 +2014,7 @@ ira_get_dup_out_num (int op_num, HARD_RE decrease_live_ranges_number (void) { basic_block bb; - rtx insn, set, src, dest, dest_death, p, q, note; + rtx set, src, dest, dest_death, p, q, note; int sregno, dregno; if (! flag_expensive_optimizations) @@ -2248,7 +2248,6 @@ compute_regs_asm_clobbered (void) FOR_EACH_BB_FN (bb, cfun) { - rtx insn; FOR_BB_INSNS_REVERSE (bb, insn) { df_ref *def_rec; @@ -3338,7 +3337,6 @@ adjust_cleared_regs (rtx loc, const_rtx static int update_equiv_regs (void) { - rtx insn; basic_block bb; int loop_depth; bitmap cleared_regs; @@ -3373,7 +3371,7 @@ update_equiv_regs (void) { loop_depth = bb_loop_depth (bb); - for (insn = BB_HEAD (bb); + for (rtx insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb)); insn = NEXT_INSN (insn)) { @@ -3593,7 +3591,7 @@ update_equiv_regs (void) /* A second pass, to gather additional equivalences with memory. This needs to be done after we know which registers we are going to replace. */ - for (insn = get_insns (); insn; insn = NEXT_INSN (insn)) + for (rtx insn = get_insns (); insn; insn = NEXT_INSN (insn)) { rtx set, src, dest; unsigned regno; @@ -3663,7 +3661,7 @@ update_equiv_regs (void) FOR_EACH_BB_REVERSE_FN (bb, cfun) { loop_depth = bb_loop_depth (bb); - for (insn = BB_END (bb); + for (rtx insn = BB_END (bb); insn != PREV_INSN (BB_HEAD (bb)); insn = PREV_INSN (insn)) { @@ -3805,7 +3803,7 @@ update_equiv_regs (void) /* Last pass - adjust debug insns referencing cleared regs. */ if (MAY_HAVE_DEBUG_INSNS) - for (insn = get_insns (); insn; insn = NEXT_INSN (insn)) + for (rtx insn = get_insns (); insn; insn = NEXT_INSN (insn)) if (DEBUG_INSN_P (insn)) { rtx old_loc = INSN_VAR_LOCATION_LOC (insn); @@ -4018,7 +4016,6 @@ build_insn_chain (void) FOR_EACH_BB_REVERSE_FN (bb, cfun) { bitmap_iterator bi; - rtx insn; CLEAR_REG_SET (live_relevant_regs); bitmap_clear (live_subregs_used); @@ -4216,7 +4213,7 @@ build_insn_chain (void) /* FIXME!! The following code is a disaster. Reload needs to see the labels and jump tables that are just hanging out in between the basic blocks. See pr33676. */ - insn = BB_HEAD (bb); + rtx insn = BB_HEAD (bb); /* Skip over the barriers and cruft. */ while (insn && (BARRIER_P (insn) || NOTE_P (insn) @@ -4422,7 +4419,6 @@ find_moveable_pseudos (void) bitmap_initialize (&unusable_as_input, 0); FOR_EACH_BB_FN (bb, cfun) { - rtx insn; bitmap transp = bb_transp_live + bb->index; bitmap moveable = bb_moveable_reg_sets + bb->index; bitmap local = bb_local + bb->index; @@ -4486,7 +4482,6 @@ find_moveable_pseudos (void) FOR_EACH_BB_FN (bb, cfun) { bitmap local = bb_local + bb->index; - rtx insn; FOR_BB_INSNS (bb, insn) if (NONDEBUG_INSN_P (insn)) @@ -4798,7 +4793,7 @@ split_live_ranges_for_shrink_wrap (void) { basic_block bb, call_dom = NULL; basic_block first = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); - rtx insn, last_interesting_insn = NULL; + rtx last_interesting_insn = NULL; bitmap_head need_new, reachable; vec<basic_block> queue; Index: gcc/jump.c =================================================================== --- gcc/jump.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/jump.c 2014-06-07 18:51:38.414968422 +0100 @@ -269,8 +269,6 @@ maybe_propagate_label_ref (rtx jump_insn static void mark_all_labels (rtx f) { - rtx insn; - if (current_ir_type () == IR_RTL_CFGLAYOUT) { basic_block bb; @@ -289,10 +287,10 @@ mark_all_labels (rtx f) /* In cfglayout mode, there may be non-insns between the basic blocks. If those non-insns represent tablejump data, they contain label references that we must record. */ - for (insn = BB_HEADER (bb); insn; insn = NEXT_INSN (insn)) + for (rtx insn = BB_HEADER (bb); insn; insn = NEXT_INSN (insn)) if (JUMP_TABLE_DATA_P (insn)) mark_jump_label (PATTERN (insn), insn, 0); - for (insn = BB_FOOTER (bb); insn; insn = NEXT_INSN (insn)) + for (rtx insn = BB_FOOTER (bb); insn; insn = NEXT_INSN (insn)) if (JUMP_TABLE_DATA_P (insn)) mark_jump_label (PATTERN (insn), insn, 0); } @@ -300,7 +298,7 @@ mark_all_labels (rtx f) else { rtx prev_nonjump_insn = NULL; - for (insn = f; insn; insn = NEXT_INSN (insn)) + for (rtx insn = f; insn; insn = NEXT_INSN (insn)) { if (INSN_DELETED_P (insn)) ; Index: gcc/haifa-sched.c =================================================================== --- gcc/haifa-sched.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/haifa-sched.c 2014-06-07 18:51:38.387968211 +0100 @@ -1037,7 +1037,6 @@ setup_ref_regs (rtx x) initiate_bb_reg_pressure_info (basic_block bb) { unsigned int i ATTRIBUTE_UNUSED; - rtx insn; if (current_nr_blocks > 1) FOR_BB_INSNS (bb, insn) @@ -8423,12 +8422,8 @@ sched_init_luids (bb_vec_t bbs) sched_extend_luids (); FOR_EACH_VEC_ELT (bbs, i, bb) - { - rtx insn; - - FOR_BB_INSNS (bb, insn) - sched_init_insn_luid (insn); - } + FOR_BB_INSNS (bb, insn) + sched_init_insn_luid (insn); } /* Free LUIDs. */ @@ -8493,12 +8488,8 @@ haifa_init_h_i_d (bb_vec_t bbs) extend_h_i_d (); FOR_EACH_VEC_ELT (bbs, i, bb) - { - rtx insn; - - FOR_BB_INSNS (bb, insn) - init_h_i_d (insn); - } + FOR_BB_INSNS (bb, insn) + init_h_i_d (insn); } /* Finalize haifa_insn_data. */ Index: gcc/loop-invariant.c =================================================================== --- gcc/loop-invariant.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/loop-invariant.c 2014-06-07 18:51:38.392968250 +0100 @@ -572,7 +572,6 @@ find_exits (struct loop *loop, basic_blo edge e; struct loop *outermost_exit = loop, *aexit; bool has_call = false; - rtx insn; for (i = 0; i < loop->num_nodes; i++) { @@ -947,8 +946,6 @@ find_invariants_insn (rtx insn, bool alw static void find_invariants_bb (basic_block bb, bool always_reached, bool always_executed) { - rtx insn; - FOR_BB_INSNS (bb, insn) { if (!NONDEBUG_INSN_P (insn)) @@ -1804,7 +1801,7 @@ calculate_loop_reg_pressure (void) unsigned int j; bitmap_iterator bi; basic_block bb; - rtx insn, link; + rtx link; struct loop *loop, *parent; FOR_EACH_LOOP (loop, 0) Index: gcc/loop-iv.c =================================================================== --- gcc/loop-iv.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/loop-iv.c 2014-06-07 18:51:38.415968429 +0100 @@ -1871,7 +1871,7 @@ eliminate_implied_conditions (enum rtx_c simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr) { bool expression_valid; - rtx head, tail, insn, cond_list, last_valid_expr; + rtx head, tail, cond_list, last_valid_expr; rtx neutral, aggr; regset altered, this_altered; edge e; @@ -1951,7 +1951,7 @@ simplify_using_initial_values (struct lo cond_list = NULL_RTX; while (1) { - insn = BB_END (e->src); + rtx insn = BB_END (e->src); if (any_condjump_p (insn)) { rtx cond = get_condition (BB_END (e->src), NULL, false, true); Index: gcc/lower-subreg.c =================================================================== --- gcc/lower-subreg.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lower-subreg.c 2014-06-07 18:51:38.393968258 +0100 @@ -1465,8 +1465,6 @@ decompose_multiword_subregs (bool decomp speed_p = optimize_function_for_speed_p (cfun); FOR_EACH_BB_FN (bb, cfun) { - rtx insn; - FOR_BB_INSNS (bb, insn) { rtx set; @@ -1545,8 +1543,6 @@ decompose_multiword_subregs (bool decomp FOR_EACH_BB_FN (bb, cfun) { - rtx insn; - FOR_BB_INSNS (bb, insn) { rtx pat; Index: gcc/lra-constraints.c =================================================================== --- gcc/lra-constraints.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra-constraints.c 2014-06-07 18:51:38.416968437 +0100 @@ -4979,8 +4979,6 @@ add_to_inherit (int regno, rtx insns) static rtx get_last_insertion_point (basic_block bb) { - rtx insn; - FOR_BB_INSNS_REVERSE (bb, insn) if (NONDEBUG_INSN_P (insn) || NOTE_INSN_BASIC_BLOCK_P (insn)) return insn; Index: gcc/lra-eliminations.c =================================================================== --- gcc/lra-eliminations.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra-eliminations.c 2014-06-07 18:51:38.393968258 +0100 @@ -1290,7 +1290,6 @@ init_elimination (void) { bool stop_to_sp_elimination_p; basic_block bb; - rtx insn; struct elim_table *ep; init_elim_table (); Index: gcc/lra.c =================================================================== --- gcc/lra.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra.c 2014-06-07 18:51:38.394968266 +0100 @@ -1809,7 +1809,7 @@ remove_scratches (void) int i; bool insn_changed_p; basic_block bb; - rtx insn, reg; + rtx reg; sloc_t loc; lra_insn_recog_data_t id; struct lra_static_insn_data *static_id; @@ -1903,7 +1903,6 @@ restore_scratches (void) check_rtl (bool final_p) { basic_block bb; - rtx insn; lra_assert (! final_p || reload_completed); FOR_EACH_BB_FN (bb, cfun) @@ -2020,7 +2019,6 @@ update_inc_notes (void) { rtx *pnote; basic_block bb; - rtx insn; FOR_EACH_BB_FN (bb, cfun) FOR_BB_INSNS (bb, insn) Index: gcc/mode-switching.c =================================================================== --- gcc/mode-switching.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/mode-switching.c 2014-06-07 18:51:38.395968273 +0100 @@ -452,7 +452,6 @@ create_pre_exit (int n_entities, int *en static int optimize_mode_switching (void) { - rtx insn; int e; basic_block bb; int need_commit = 0; Index: gcc/postreload-gcse.c =================================================================== --- gcc/postreload-gcse.c 2014-06-07 18:23:27.320167705 +0100 +++ gcc/postreload-gcse.c 2014-06-07 18:51:38.395968273 +0100 @@ -261,7 +261,6 @@ alloc_mem (void) { int i; basic_block bb; - rtx insn; /* Find the largest UID and create a mapping from UIDs to CUIDs. */ uid_cuid = XCNEWVEC (int, get_max_uid () + 1); @@ -830,8 +829,6 @@ compute_hash_table (void) FOR_EACH_BB_FN (bb, cfun) { - rtx insn; - /* First pass over the instructions records information used to determine when registers and memory are last set. Since we compute a "local" AVAIL_OUT, reset the tables that @@ -839,10 +836,8 @@ compute_hash_table (void) of the block. */ reset_opr_set_tables (); FOR_BB_INSNS (bb, insn) - { - if (INSN_P (insn)) - record_opr_changes (insn); - } + if (INSN_P (insn)) + record_opr_changes (insn); /* The next pass actually builds the hash table. */ FOR_BB_INSNS (bb, insn) @@ -1153,7 +1148,6 @@ eliminate_partially_redundant_load (basi static void eliminate_partially_redundant_loads (void) { - rtx insn; basic_block bb; /* Note we start at block 1. */ Index: gcc/postreload.c =================================================================== --- gcc/postreload.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/postreload.c 2014-06-07 18:51:38.396968281 +0100 @@ -207,7 +207,6 @@ reload_cse_regs_1 (void) { bool cfg_changed = false; basic_block bb; - rtx insn; rtx testreg = gen_rtx_REG (VOIDmode, -1); cselib_init (CSELIB_RECORD_MEMORY); Index: gcc/predict.c =================================================================== --- gcc/predict.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/predict.c 2014-06-07 18:51:38.397968289 +0100 @@ -2890,17 +2890,13 @@ expensive_function_p (int threshold) /* Maximally BB_FREQ_MAX^2 so overflow won't happen. */ limit = ENTRY_BLOCK_PTR_FOR_FN (cfun)->frequency * threshold; FOR_EACH_BB_FN (bb, cfun) - { - rtx insn; - - FOR_BB_INSNS (bb, insn) - if (active_insn_p (insn)) - { - sum += bb->frequency; - if (sum > limit) - return true; + FOR_BB_INSNS (bb, insn) + if (active_insn_p (insn)) + { + sum += bb->frequency; + if (sum > limit) + return true; } - } return false; } Index: gcc/ree.c =================================================================== --- gcc/ree.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/ree.c 2014-06-07 18:51:38.397968289 +0100 @@ -982,7 +982,7 @@ find_removable_extensions (void) { vec<ext_cand> insn_list = vNULL; basic_block bb; - rtx insn, set; + rtx set; unsigned *def_map = XCNEWVEC (unsigned, max_insn_uid); FOR_EACH_BB_FN (bb, cfun) Index: gcc/reginfo.c =================================================================== --- gcc/reginfo.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/reginfo.c 2014-06-07 18:51:38.398968297 +0100 @@ -1257,7 +1257,6 @@ find_subregs_of_mode (rtx x, bitmap subr init_subregs_of_mode (void) { basic_block bb; - rtx insn; bitmap_obstack srom_obstack; bitmap subregs_of_mode; Index: gcc/regrename.c =================================================================== --- gcc/regrename.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/regrename.c 2014-06-07 18:51:38.398968297 +0100 @@ -724,7 +724,6 @@ regrename_analyze (bitmap bb_mask) open_chains = NULL; if (insn_rr.exists ()) { - rtx insn; FOR_BB_INSNS (bb1, insn) { insn_rr_info *p = &insn_rr[INSN_UID (insn)]; Index: gcc/regstat.c =================================================================== --- gcc/regstat.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/regstat.c 2014-06-07 18:51:38.417968445 +0100 @@ -121,7 +121,6 @@ regstat_bb_compute_ri (unsigned int bb_i int *local_live_last_luid) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; df_ref *def_rec; df_ref *use_rec; int luid = 0; @@ -441,7 +440,6 @@ regstat_get_setjmp_crosses (void) regstat_bb_compute_calls_crossed (unsigned int bb_index, bitmap live) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, bb_index); - rtx insn; df_ref *def_rec; df_ref *use_rec; Index: gcc/reload1.c =================================================================== --- gcc/reload1.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/reload1.c 2014-06-07 18:51:38.400968312 +0100 @@ -1605,7 +1605,6 @@ calculate_elim_costs_all_insns (void) FOR_EACH_BB_FN (bb, cfun) { - rtx insn; elim_bb = bb; FOR_BB_INSNS (bb, insn) Index: gcc/sched-rgn.c =================================================================== --- gcc/sched-rgn.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/sched-rgn.c 2014-06-07 18:51:38.401968320 +0100 @@ -256,7 +256,6 @@ static void free_pending_lists (void); is_cfg_nonregular (void) { basic_block b; - rtx insn; /* If we have a label that could be the target of a nonlocal goto, then the cfg is not well structured. */ @@ -538,13 +537,9 @@ rgn_estimate_number_of_insns (basic_bloc count = INSN_LUID (BB_END (bb)) - INSN_LUID (BB_HEAD (bb)); if (MAY_HAVE_DEBUG_INSNS) - { - rtx insn; - - FOR_BB_INSNS (bb, insn) - if (DEBUG_INSN_P (insn)) - count--; - } + FOR_BB_INSNS (bb, insn) + if (DEBUG_INSN_P (insn)) + count--; return count; } Index: gcc/sched-vis.c =================================================================== --- gcc/sched-vis.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/sched-vis.c 2014-06-07 18:51:38.401968320 +0100 @@ -832,7 +832,6 @@ dump_rtl_slim (FILE *f, const_rtx first, void rtl_dump_bb_for_graph (pretty_printer *pp, basic_block bb) { - rtx insn; bool first = true; /* TODO: inter-bb stuff. */ Index: gcc/sel-sched-ir.c =================================================================== --- gcc/sel-sched-ir.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/sel-sched-ir.c 2014-06-07 18:51:38.402968328 +0100 @@ -2801,12 +2801,8 @@ sched_scan (const struct sched_scan_info if (ssi->init_insn) FOR_EACH_VEC_ELT (bbs, i, bb) - { - rtx insn; - - FOR_BB_INSNS (bb, insn) - ssi->init_insn (insn); - } + FOR_BB_INSNS (bb, insn) + ssi->init_insn (insn); } /* Implement hooks for collecting fundamental insn properties like if insn is @@ -4647,7 +4643,6 @@ sel_init_bbs (bb_vec_t bbs) sel_restore_notes (void) { int bb; - insn_t insn; for (bb = 0; bb < current_nr_blocks; bb++) { @@ -4949,8 +4944,6 @@ recompute_rev_top_order (void) void clear_outdated_rtx_info (basic_block bb) { - rtx insn; - FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) { @@ -5404,7 +5397,6 @@ change_loops_latches (basic_block from, sel_split_block (basic_block bb, rtx after) { basic_block new_bb; - insn_t insn; new_bb = sched_split_block_1 (bb, after); sel_add_bb (new_bb); Index: gcc/sel-sched.c =================================================================== --- gcc/sel-sched.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/sel-sched.c 2014-06-07 18:51:38.404968344 +0100 @@ -7005,7 +7005,6 @@ simplify_changed_insns (void) for (i = 0; i < current_nr_blocks; i++) { basic_block bb = BASIC_BLOCK_FOR_FN (cfun, BB_TO_BLOCK (i)); - rtx insn; FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) Index: gcc/stack-ptr-mod.c =================================================================== --- gcc/stack-ptr-mod.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/stack-ptr-mod.c 2014-06-07 18:51:38.405968351 +0100 @@ -84,7 +84,6 @@ const pass_data pass_data_stack_ptr_mod pass_stack_ptr_mod::execute (function *fun) { basic_block bb; - rtx insn; /* Assume that the stack pointer is unchanging if alloca hasn't been used. */ Index: gcc/store-motion.c =================================================================== --- gcc/store-motion.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/store-motion.c 2014-06-07 18:51:38.405968351 +0100 @@ -644,7 +644,7 @@ compute_store_table (void) #ifdef ENABLE_CHECKING unsigned regno; #endif - rtx insn, tmp; + rtx tmp; df_ref *def_rec; int *last_set_in, *already_set; struct st_expr * ptr, **prev_next_ptr_ptr; @@ -1010,7 +1010,7 @@ build_store_vectors (void) { basic_block bb; int *regs_set_in_block; - rtx insn, st; + rtx st; struct st_expr * ptr; unsigned int max_gcse_regno = max_reg_num (); @@ -1028,7 +1028,7 @@ build_store_vectors (void) { for (st = ptr->avail_stores; st != NULL; st = XEXP (st, 1)) { - insn = XEXP (st, 0); + rtx insn = XEXP (st, 0); bb = BLOCK_FOR_INSN (insn); /* If we've already seen an available expression in this block, @@ -1048,7 +1048,7 @@ build_store_vectors (void) for (st = ptr->antic_stores; st != NULL; st = XEXP (st, 1)) { - insn = XEXP (st, 0); + rtx insn = XEXP (st, 0); bb = BLOCK_FOR_INSN (insn); bitmap_set_bit (st_antloc[bb->index], ptr->index); } Index: gcc/web.c =================================================================== --- gcc/web.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/web.c 2014-06-07 18:51:38.405968351 +0100 @@ -363,7 +363,6 @@ pass_web::execute (function *fun) unsigned int *used; basic_block bb; unsigned int uses_num = 0; - rtx insn; df_set_flags (DF_NO_HARD_REGS + DF_EQ_NOTES); df_set_flags (DF_RD_PRUNE_DEAD_DEFS); Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/arm/arm.c 2014-06-07 18:51:38.412968406 +0100 @@ -17021,7 +17021,6 @@ thumb2_reorg (void) && optimize_bb_for_speed_p (bb)) continue; - rtx insn; Convert_Action action = SKIP; Convert_Action action_for_partial_flag_setting = (current_tune->disparage_partial_flag_setting_t16_encodings Index: gcc/config/epiphany/resolve-sw-modes.c =================================================================== --- gcc/config/epiphany/resolve-sw-modes.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/epiphany/resolve-sw-modes.c 2014-06-07 18:51:38.354967954 +0100 @@ -78,7 +78,7 @@ const pass_data pass_data_resolve_sw_mod pass_resolve_sw_modes::execute (function *fun) { basic_block bb; - rtx insn, src; + rtx src; vec<basic_block> todo; sbitmap pushed; bool need_commit = false; Index: gcc/config/i386/i386.c =================================================================== --- gcc/config/i386/i386.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/i386/i386.c 2014-06-07 18:51:38.366968047 +0100 @@ -10676,7 +10676,6 @@ ix86_finalize_stack_realign_flags (void) HARD_FRAME_POINTER_REGNUM); FOR_EACH_BB_FN (bb, cfun) { - rtx insn; FOR_BB_INSNS (bb, insn) if (NONDEBUG_INSN_P (insn) && requires_stack_frame_p (insn, prologue_used, @@ -39233,7 +39232,6 @@ ix86_pad_returns (void) static int ix86_count_insn_bb (basic_block bb) { - rtx insn; int insn_count = 0; /* Count number of instructions in this block. Return 4 if the number Index: gcc/config/mips/mips.c =================================================================== --- gcc/config/mips/mips.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/mips/mips.c 2014-06-07 18:51:38.371968087 +0100 @@ -15516,30 +15516,29 @@ mips_get_pic_call_symbol (rtx *operands, mips_annotate_pic_calls (void) { basic_block bb; - rtx insn; FOR_EACH_BB_FN (bb, cfun) FOR_BB_INSNS (bb, insn) - { - rtx call, reg, symbol, second_call; + { + rtx call, reg, symbol, second_call; - second_call = 0; - call = mips_call_expr_from_insn (insn, &second_call); - if (!call) - continue; - gcc_assert (MEM_P (XEXP (call, 0))); - reg = XEXP (XEXP (call, 0), 0); - if (!REG_P (reg)) - continue; + second_call = 0; + call = mips_call_expr_from_insn (insn, &second_call); + if (!call) + continue; + gcc_assert (MEM_P (XEXP (call, 0))); + reg = XEXP (XEXP (call, 0), 0); + if (!REG_P (reg)) + continue; - symbol = mips_find_pic_call_symbol (insn, reg, true); - if (symbol) - { - mips_annotate_pic_call_expr (call, symbol); - if (second_call) - mips_annotate_pic_call_expr (second_call, symbol); - } - } + symbol = mips_find_pic_call_symbol (insn, reg, true); + if (symbol) + { + mips_annotate_pic_call_expr (call, symbol); + if (second_call) + mips_annotate_pic_call_expr (second_call, symbol); + } + } } /* A temporary variable used by for_each_rtx callbacks, etc. */ Index: gcc/config/mn10300/mn10300.c =================================================================== --- gcc/config/mn10300/mn10300.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/mn10300/mn10300.c 2014-06-07 18:51:38.372968094 +0100 @@ -3216,8 +3216,6 @@ mn10300_insert_setlb_lcc (rtx label, rtx static bool mn10300_block_contains_call (basic_block block) { - rtx insn; - FOR_BB_INSNS (block, insn) if (CALL_P (insn)) return true; Index: gcc/config/s390/s390.c =================================================================== --- gcc/config/s390/s390.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/s390/s390.c 2014-06-07 18:51:38.375968118 +0100 @@ -7419,7 +7419,6 @@ s390_reg_clobbered_rtx (rtx setreg, cons s390_regs_ever_clobbered (char regs_ever_clobbered[]) { basic_block cur_bb; - rtx cur_insn; unsigned int i; memset (regs_ever_clobbered, 0, 32); @@ -7968,7 +7967,6 @@ s390_optimize_nonescaping_tx (void) basic_block tbegin_bb = NULL; basic_block tend_bb = NULL; basic_block bb; - rtx insn; bool result = true; int bb_index; rtx tbegin_insn = NULL_RTX; Index: gcc/lra-lives.c =================================================================== --- gcc/lra-lives.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra-lives.c 2014-06-07 18:51:38.417968445 +0100 @@ -519,21 +519,22 @@ process_bb_lives (basic_block bb, int &c FOO will remain live until the beginning of the block. Likewise if FOO is not set at all. This is unnecessarily pessimistic, but it probably doesn't matter much in practice. */ - FOR_BB_INSNS_REVERSE (bb, curr_insn) + FOR_BB_INSNS_REVERSE (bb, insn) { bool call_p; int dst_regno, src_regno; rtx set; struct lra_insn_reg *reg; - if (!NONDEBUG_INSN_P (curr_insn)) + if (!NONDEBUG_INSN_P (insn)) continue; - curr_id = lra_get_insn_recog_data (curr_insn); + curr_insn = insn; + curr_id = lra_get_insn_recog_data (insn); curr_static_id = curr_id->insn_static_data; if (lra_dump_file != NULL) fprintf (lra_dump_file, " Insn %u: point = %d\n", - INSN_UID (curr_insn), curr_point); + INSN_UID (insn), curr_point); /* Update max ref width and hard reg usage. */ for (reg = curr_id->regs; reg != NULL; reg = reg->next) @@ -544,9 +545,9 @@ process_bb_lives (basic_block bb, int &c else if (reg->regno < FIRST_PSEUDO_REGISTER) lra_hard_reg_usage[reg->regno] += freq; - call_p = CALL_P (curr_insn); + call_p = CALL_P (insn); if (complete_info_p - && (set = single_set (curr_insn)) != NULL_RTX + && (set = single_set (insn)) != NULL_RTX && REG_P (SET_DEST (set)) && REG_P (SET_SRC (set)) /* Check that source regno does not conflict with destination regno to exclude most impossible @@ -627,7 +628,7 @@ process_bb_lives (basic_block bb, int &c if (flag_use_caller_save) { HARD_REG_SET this_call_used_reg_set; - get_call_reg_set_usage (curr_insn, &this_call_used_reg_set, + get_call_reg_set_usage (insn, &this_call_used_reg_set, call_used_reg_set); EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, j) @@ -638,7 +639,7 @@ process_bb_lives (basic_block bb, int &c sparseset_ior (pseudos_live_through_calls, pseudos_live_through_calls, pseudos_live); if (cfun->has_nonlocal_label - || find_reg_note (curr_insn, REG_SETJMP, + || find_reg_note (insn, REG_SETJMP, NULL_RTX) != NULL_RTX) sparseset_ior (pseudos_live_through_setjumps, pseudos_live_through_setjumps, pseudos_live); @@ -688,7 +689,7 @@ process_bb_lives (basic_block bb, int &c next_program_point (curr_point, freq); /* Update notes. */ - for (link_loc = ®_NOTES (curr_insn); (link = *link_loc) != NULL_RTX;) + for (link_loc = ®_NOTES (insn); (link = *link_loc) != NULL_RTX;) { if (REG_NOTE_KIND (link) != REG_DEAD && REG_NOTE_KIND (link) != REG_UNUSED) @@ -712,9 +713,9 @@ process_bb_lives (basic_block bb, int &c link_loc = &XEXP (link, 1); } EXECUTE_IF_SET_IN_SPARSESET (dead_set, j) - add_reg_note (curr_insn, REG_DEAD, regno_reg_rtx[j]); + add_reg_note (insn, REG_DEAD, regno_reg_rtx[j]); EXECUTE_IF_SET_IN_SPARSESET (unused_set, j) - add_reg_note (curr_insn, REG_UNUSED, regno_reg_rtx[j]); + add_reg_note (insn, REG_UNUSED, regno_reg_rtx[j]); } #ifdef EH_RETURN_DATA_REGNO Index: gcc/loop-unroll.c =================================================================== --- gcc/loop-unroll.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/loop-unroll.c 2014-06-07 18:51:38.392968250 +0100 @@ -324,7 +324,6 @@ unroll_and_peel_loops (int flags) loop_exit_at_end_p (struct loop *loop) { struct niter_desc *desc = get_simple_loop_desc (loop); - rtx insn; if (desc->in_edge->dest != loop->latch) return false; @@ -1689,7 +1688,6 @@ referenced_in_one_insn_in_loop_p (struct basic_block *body, bb; unsigned i; int count_ref = 0; - rtx insn; body = get_loop_body (loop); for (i = 0; i < loop->num_nodes; i++) @@ -1715,7 +1713,6 @@ reset_debug_uses_in_loop (struct loop *l { basic_block *body, bb; unsigned i; - rtx insn; body = get_loop_body (loop); for (i = 0; debug_uses && i < loop->num_nodes; i++) @@ -1959,7 +1956,6 @@ analyze_insns_in_loop (struct loop *loop basic_block *body, bb; unsigned i; struct opt_info *opt_info = XCNEW (struct opt_info); - rtx insn; struct iv_to_split *ivts = NULL; struct var_to_expand *ves = NULL; iv_to_split **slot1; @@ -2398,7 +2394,7 @@ apply_opt_in_copies (struct opt_info *op { unsigned i, delta; basic_block bb, orig_bb; - rtx insn, orig_insn, next; + rtx orig_insn, next; struct iv_to_split ivts_templ, *ivts; struct var_to_expand ve_templ, *ves; @@ -2424,7 +2420,7 @@ apply_opt_in_copies (struct opt_info *op unrolling); bb->aux = 0; orig_insn = BB_HEAD (orig_bb); - FOR_BB_INSNS_SAFE (bb, insn, next) + FOR_BB_INSNS (bb, insn) { if (!INSN_P (insn) || (DEBUG_INSN_P (insn) Index: gcc/auto-inc-dec.c =================================================================== --- gcc/auto-inc-dec.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/auto-inc-dec.c 2014-06-07 18:51:38.406968359 +0100 @@ -1333,14 +1333,12 @@ find_mem (rtx *address_of_x) static void merge_in_block (int max_reg, basic_block bb) { - rtx insn; - rtx curr; int success_in_block = 0; if (dump_file) fprintf (dump_file, "\n\nstarting bb %d\n", bb->index); - FOR_BB_INSNS_REVERSE_SAFE (bb, insn, curr) + FOR_BB_INSNS_REVERSE (bb, insn) { unsigned int uid = INSN_UID (insn); bool insn_is_add_or_inc = true; Index: gcc/lra-coalesce.c =================================================================== --- gcc/lra-coalesce.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra-coalesce.c 2014-06-07 18:51:38.415968429 +0100 @@ -219,7 +219,7 @@ coalescable_pseudo_p (int regno) lra_coalesce (void) { basic_block bb; - rtx mv, set, insn, next, *sorted_moves; + rtx mv, set, *sorted_moves; int i, mv_num, sregno, dregno; unsigned int regno; int coalesced_moves; @@ -244,7 +244,7 @@ lra_coalesce (void) coalesced_moves = 0; FOR_EACH_BB_FN (bb, cfun) { - FOR_BB_INSNS_SAFE (bb, insn, next) + FOR_BB_INSNS (bb, insn) if (INSN_P (insn) && (set = single_set (insn)) != NULL_RTX && REG_P (SET_DEST (set)) && REG_P (SET_SRC (set)) @@ -304,7 +304,7 @@ lra_coalesce (void) { update_live_info (df_get_live_in (bb)); update_live_info (df_get_live_out (bb)); - FOR_BB_INSNS_SAFE (bb, insn, next) + FOR_BB_INSNS (bb, insn) if (INSN_P (insn) && bitmap_bit_p (&involved_insns_bitmap, INSN_UID (insn))) { Index: gcc/var-tracking.c =================================================================== --- gcc/var-tracking.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/var-tracking.c 2014-06-07 18:51:38.419968460 +0100 @@ -10182,14 +10182,13 @@ static int debug_label_num = 1; delete_debug_insns (void) { basic_block bb; - rtx insn, next; if (!MAY_HAVE_DEBUG_INSNS) return; FOR_EACH_BB_FN (bb, cfun) { - FOR_BB_INSNS_SAFE (bb, insn, next) + FOR_BB_INSNS (bb, insn) if (DEBUG_INSN_P (insn)) { tree decl = INSN_VAR_LOCATION_DECL (insn); Index: gcc/shrink-wrap.c =================================================================== --- gcc/shrink-wrap.c 2014-06-07 18:23:27.320167705 +0100 +++ gcc/shrink-wrap.c 2014-06-07 18:51:38.404968344 +0100 @@ -331,7 +331,7 @@ move_insn_for_shrink_wrap (basic_block b void prepare_shrink_wrap (basic_block entry_block) { - rtx insn, curr, x; + rtx x; HARD_REG_SET uses, defs; df_ref *ref; bool split_p = false; @@ -347,7 +347,7 @@ prepare_shrink_wrap (basic_block entry_b CLEAR_HARD_REG_SET (uses); CLEAR_HARD_REG_SET (defs); - FOR_BB_INSNS_REVERSE_SAFE (entry_block, insn, curr) + FOR_BB_INSNS_REVERSE (entry_block, insn) if (NONDEBUG_INSN_P (insn) && !move_insn_for_shrink_wrap (entry_block, insn, uses, defs, &split_p)) @@ -513,7 +513,6 @@ try_shrink_wrapping (edge *entry_edge, e FOR_EACH_BB_FN (bb, cfun) { - rtx insn; unsigned size = 0; FOR_BB_INSNS (bb, insn) Index: gcc/dce.c =================================================================== --- gcc/dce.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/dce.c 2014-06-07 18:51:38.413968413 +0100 @@ -509,10 +509,9 @@ remove_reg_equal_equiv_notes_for_defs (r reset_unmarked_insns_debug_uses (void) { basic_block bb; - rtx insn, next; FOR_EACH_BB_REVERSE_FN (bb, cfun) - FOR_BB_INSNS_REVERSE_SAFE (bb, insn, next) + FOR_BB_INSNS_REVERSE (bb, insn) if (DEBUG_INSN_P (insn)) { df_ref *use_rec; @@ -547,11 +546,10 @@ reset_unmarked_insns_debug_uses (void) delete_unmarked_insns (void) { basic_block bb; - rtx insn, next; bool must_clean = false; FOR_EACH_BB_REVERSE_FN (bb, cfun) - FOR_BB_INSNS_REVERSE_SAFE (bb, insn, next) + FOR_BB_INSNS_REVERSE (bb, insn) if (NONDEBUG_INSN_P (insn)) { /* Always delete no-op moves. */ @@ -614,7 +612,6 @@ delete_unmarked_insns (void) prescan_insns_for_dce (bool fast) { basic_block bb; - rtx insn, prev; bitmap arg_stores = NULL; if (dump_file) @@ -625,7 +622,7 @@ prescan_insns_for_dce (bool fast) FOR_EACH_BB_FN (bb, cfun) { - FOR_BB_INSNS_REVERSE_SAFE (bb, insn, prev) + FOR_BB_INSNS_REVERSE (bb, insn) if (NONDEBUG_INSN_P (insn)) { /* Don't mark argument stores now. They will be marked @@ -839,7 +836,6 @@ word_dce_process_block (basic_block bb, struct dead_debug_global *global_debug) { bitmap local_live = BITMAP_ALLOC (&dce_tmp_bitmap_obstack); - rtx insn; bool block_changed; struct dead_debug_local debug; @@ -937,7 +933,6 @@ dce_process_block (basic_block bb, bool struct dead_debug_global *global_debug) { bitmap local_live = BITMAP_ALLOC (&dce_tmp_bitmap_obstack); - rtx insn; bool block_changed; df_ref *def_rec; struct dead_debug_local debug; Index: gcc/lra-spills.c =================================================================== --- gcc/lra-spills.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/lra-spills.c 2014-06-07 18:51:38.394968266 +0100 @@ -256,7 +256,7 @@ assign_spill_hard_regs (int *pseudo_regn enum reg_class rclass, spill_class; enum machine_mode mode; lra_live_range_t r; - rtx insn, set; + rtx set; basic_block bb; HARD_REG_SET conflict_hard_regs; bitmap_head ok_insn_bitmap; @@ -463,7 +463,6 @@ remove_pseudos (rtx *loc, rtx insn) spill_pseudos (void) { basic_block bb; - rtx insn; int i; bitmap_head spilled_pseudos, changed_insns; @@ -679,7 +678,6 @@ lra_final_code_change (void) { int i, hard_regno; basic_block bb; - rtx insn, curr; int max_regno = max_reg_num (); for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++) @@ -687,7 +685,7 @@ lra_final_code_change (void) && (hard_regno = lra_get_regno_hard_regno (i)) >= 0) SET_REGNO (regno_reg_rtx[i], hard_regno); FOR_EACH_BB_FN (bb, cfun) - FOR_BB_INSNS_SAFE (bb, insn, curr) + FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) { rtx pat = PATTERN (insn); Index: gcc/config/c6x/c6x.c =================================================================== --- gcc/config/c6x/c6x.c 2014-06-07 18:23:27.319167704 +0100 +++ gcc/config/c6x/c6x.c 2014-06-07 18:51:38.353967946 +0100 @@ -5383,7 +5383,6 @@ split_delayed_insns (void) conditionalize_after_sched (void) { basic_block bb; - rtx insn; FOR_EACH_BB_FN (bb, cfun) FOR_BB_INSNS (bb, insn) { @@ -5427,7 +5426,6 @@ hwloop_pattern_reg (rtx insn) bb_earliest_end_cycle (basic_block bb, rtx ignore) { int earliest = 0; - rtx insn; FOR_BB_INSNS (bb, insn) { @@ -5453,11 +5451,10 @@ bb_earliest_end_cycle (basic_block bb, r static void filter_insns_above (basic_block bb, int max_uid) { - rtx insn, next; bool prev_ti = false; int prev_cycle = -1; - FOR_BB_INSNS_SAFE (bb, insn, next) + FOR_BB_INSNS (bb, insn) { int this_cycle; if (!NONDEBUG_INSN_P (insn))
I noticed the head of FOR_BB_INSNS loops showing up high in the profile, both for -O0 and -O2, so this patch tries to make the loops more efficient. The current definition of FOR_BB_INSNS is: #define FOR_BB_INSNS(BB, INSN) \ for ((INSN) = BB_HEAD (BB); \ (INSN) && (INSN) != NEXT_INSN (BB_END (BB)); \ (INSN) = NEXT_INSN (INSN)) 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. 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, then we could cache the end point in the loop header. I think that restriction already applies to the special case of INSN == BB_END (bb) in FOR_BB_INSNS_SAFE: #define FOR_BB_INSNS_SAFE(BB, INSN, CURR) \ for ((INSN) = BB_HEAD (BB), (CURR) = (INSN) ? NEXT_INSN ((INSN)): NULL; \ (INSN) && (INSN) != NEXT_INSN (BB_END (BB)); \ (INSN) = (CURR), (CURR) = (INSN) ? NEXT_INSN ((INSN)) : NULL) because otherwise CURR would end up being after NEXT_INSN (BB_END (bb)) and the loop wouldn't terminate properly. It's easier to change these macros if they define the INSN variables themselves. Also, the modified version of FOR_BB_INSNS_SAFE ended up being significantly faster than the modified version of FOR_BB_INSNS. I.e. fetching NEXT_INSN in the loop header is better than fetching at continuation time. FOR_BB_INSNS_SAFE also has the (IMO) nice property of skipping instructions that the loop body inserts immediately after INSN, just like it would skip those that the body inserts immediately before INSN. I tried to check for loops that might be relying on the old behaviour but I couldn't see any. So the patch also gets rid of the safe/unsafe distinction and makes the normal iterators "safe". This seems to give a consistent 1% speedup on both -O0 and -O2 compiles that I've tried. Tested on x86_64-linux-gnu. I also checked that there were no changes in asm output for gcc.dg, g++.dg and gcc.c-torture for x86_64 and that one target from each directory still builds correctly. OK to install? Thanks, Richard gcc/ * basic-block.h (FOR_BB_INSNS): Redefine as a "safe" iterator that declares the iteration variable itself. Cache the value of the terminator. Make the compiler aware that the loop iterates at least once if the first insn is nonnull. (FOR_BB_INSNS_REVERSE): Likewise. (FOR_BB_INSNS_SAFE, FOR_BB_INSNS_REVERSE_SAFE): Delete. * alias.c (init_alias_analysis): Remove separate variable declarations. * bb-reorder.c (copy_bb_p): Likewise. (pass_duplicate_computed_gotos::execute): Likewise. * cfgloop.c (get_loop_location): Likewise. * cfgloopanal.c (num_loop_insns, average_num_loop_insns): Likewise. * cfgrtl.c (rtl_verify_bb_pointers, rtl_block_empty_p): Likewise. (rtl_split_block_before_cond_jump): Likewise. (rtl_account_profile_record): Likewise. * combine.c (create_log_links, combine_instructions): Likewise. * cprop.c (compute_hash_table_work, local_cprop_pass): Likewise. (bypass_conditional_jumps, one_cprop_pass): Likewise. * cse.c (cse_prescan_path, cse_extended_basic_block): Likewise. * df-core.c (df_bb_regno_first_def_find): Likewise. (df_bb_regno_last_def_find): Likewise. * df-problems.c (df_rd_bb_local_compute): Likewise. (df_lr_bb_local_compute, df_live_bb_local_compute): Likewise. (df_chain_remove_problem, df_chain_create_bb): Likewise. (df_word_lr_bb_local_compute, df_note_bb_compute): Likewise. (df_md_bb_local_compute): Likewise. * df-scan.c (df_scan_free_bb_info, df_scan_start_dump): Likewise. (df_reorganize_refs_by_reg_by_insn): Likewise. (df_reorganize_refs_by_insn_bb, df_recompute_luids): Likewise. (df_bb_refs_record, df_bb_verify): Likewise. (df_insn_rescan_all, df_update_entry_exit_and_calls): Likewise (with reindentation). * dse.c (dse_step1): Likewise. * function.c (reposition_prologue_and_epilogue_notes): Likewise. (pass_match_asm_constraints::execute): Likewise. * fwprop.c (single_def_use_dom_walker::before_dom_children): Likewise. * gcse.c (compute_hash_table_work, hoist_code): Likewise. (calculate_bb_reg_pressure, compute_ld_motion_mems): Likewise. * ifcvt.c (noce_can_store_speculate_p): Likewise. (cond_move_convert_if_block, dead_or_predicable): Likewise. * init-regs.c (initialize_uninitialized_regs): Likewise. * ira-build.c (create_bb_allocnos): Likewise. * ira-conflicts.c (add_copies): Likewise. * ira-costs.c (process_bb_for_costs): Likewise. (process_bb_node_for_hard_reg_moves): Likewise. * ira-emit.c (change_loop, ira_emit): Likewise. * ira-lives.c (process_bb_node_lives): Likewise. * ira.c (decrease_live_ranges_number): Likewise. (compute_regs_asm_clobbered, update_equiv_regs): Likewise. (build_insn_chain, find_moveable_pseudos): Likewise. (split_live_ranges_for_shrink_wrap): Likewise. * jump.c (mark_all_labels): Likewise. * haifa-sched.c (initiate_bb_reg_pressure_info): Likewise. (sched_init_luids, haifa_init_h_i_d): Likewise (with reindentation). * loop-invariant.c (find_exits, find_invariants_bb): Likewise. (calculate_loop_reg_pressure): Likewise. * loop-iv.c (simplify_using_initial_values): Likewise. * lower-subreg.c (decompose_multiword_subregs): Likewise. * lra-constraints.c (get_last_insertion_point): Likewise. * lra-eliminations.c (init_elimination): Likewise. * lra.c (remove_scratches, check_rtl, update_inc_notes): Likewise. * mode-switching.c (optimize_mode_switching): Likewise. * postreload-gcse.c (alloc_mem): Likewise. (compute_hash_table): Likewise (with reindentation). (eliminate_partially_redundant_loads): Likewise. * postreload.c (reload_cse_regs_1): Likewise. * predict.c (expensive_function_p): Likewise (with reindenation). * ree.c (find_removable_extensions): Likewise. * reginfo.c (init_subregs_of_mode): Likewise. * regrename.c (regrename_analyze): Likewise. * regstat.c (regstat_bb_compute_ri): Likewise. (regstat_bb_compute_calls_crossed): Likewise. * reload1.c (calculate_elim_costs_all_insns): Likewise. * sched-rgn.c (is_cfg_nonregular): Likewise. (rgn_estimate_number_of_insns): Likewise (with reindentation). * sched-vis.c (rtl_dump_bb_for_graph): Likewise. * sel-sched-ir.c (sched_scan): Likewise (with reindentation). (sel_restore_notes, clear_outdated_rtx_info): Likewise. (sel_split_block): Likewise. * sel-sched.c (simplify_changed_insns): Likewise. * stack-ptr-mod.c (pass_stack_ptr_mod::execute): Likewise. * store-motion.c (compute_store_table, build_store_vectors): Likewise. * web.c (pass_web::execute): Likewise. * config/arm/arm.c (thumb2_reorg): Likewise. * config/epiphany/resolve-sw-modes.c (pass_resolve_sw_modes::execute): Likewise. * config/i386/i386.c (ix86_finalize_stack_realign_flags): Likewise. (ix86_count_insn_bb): Likewise. * config/mips/mips.c (mips_get_pic_call_symbol): Likewise (with reindentation). * config/mn10300/mn10300.c (mn10300_block_contains_call): Likewise. * config/s390/s390.c (s390_regs_ever_clobbered): Likewise. (s390_optimize_nonescaping_tx): Likewise. * lra-lives.c (process_bb_lives): Use a local "insn" variable and assign it to curr_insn. * loop-unroll.c (loop_exit_at_end_p): Remove separate variable declarations. (referenced_in_one_insn_in_loop_p, reset_debug_uses_in_loop): Likewise. (analyze_insns_in_loop): Likewise. (apply_opt_in_copies): Replace use of safe iterators with standard ones. * auto-inc-dec.c (merge_in_block): Likewise. * lra-coalesce.c (lra_coalesce): Likewise. * var-tracking.c (delete_debug_insns): Likewise. * shrink-wrap.c (prepare_shrink_wrap): Likewise. (try_shrink_wrapping): Remove separate variable declarations. * dce.c (word_dce_process_block, dce_process_block): Likewise. (reset_unmarked_insns_debug_uses): Replace use of safe iterators with standard ones. (delete_unmarked_insns, prescan_insns_for_dce): Likewise. * lra-spills.c (lra_final_code_change): Likewise. (assign_spill_hard_regs, spill_pseudos): Remove separate variable declarations. * config/c6x/c6x.c (conditionalize_after_sched): Likewise. (bb_earliest_end_cycle): Likewise. (filter_insns_above): Replace use of safe iterators with standard ones.