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
