From: Steven Bosscher <stevenb.gcc@gmail.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>,
"rdsandiford@googlemail.com" <rdsandiford@googlemail.com>
Subject: Re: RFA: Rework FOR_BB_INSNS iterators
Date: Sat, 07 Jun 2014 20:26:00 -0000 [thread overview]
Message-ID: <CABu31nPovtFLwmOAuiqT5EadJZCqg+U7RqOMYAZimf3ykVE=OA@mail.gmail.com> (raw)
In-Reply-To: <87vbscppva.fsf@talisman.default>
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
next prev parent reply other threads:[~2014-06-07 20:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-07 17:54 Richard Sandiford
2014-06-07 20:26 ` Steven Bosscher [this message]
2014-06-09 19:32 ` Richard Sandiford
2014-06-23 19:01 ` Instructions vs Expressions in the backend (was Re: RFA: Rework FOR_BB_INSNS iterators) David Malcolm
2014-06-23 20:38 ` Oleg Endo
2014-06-25 9:36 ` Richard Sandiford
2014-06-25 20:39 ` Jeff Law
2014-06-27 14:28 ` David Malcolm
2014-06-27 15:38 ` Jeff Law
2014-06-27 7:36 ` Oleg Endo
2014-06-27 14:35 ` David Malcolm
2014-06-25 8:54 ` Richard Sandiford
2014-06-25 20:46 ` Jeff Law
2014-06-25 21:24 ` Steven Bosscher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABu31nPovtFLwmOAuiqT5EadJZCqg+U7RqOMYAZimf3ykVE=OA@mail.gmail.com' \
--to=stevenb.gcc@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rdsandiford@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).