On Thu, Feb 21, 2013 at 10:59 AM, Richard Biener wrote:
> On Thu, Feb 21, 2013 at 1:10 AM, Steven Bosscher wrote:
> +/* Delete all of the refs information from the insn with UID.
> + Internal helper for df_insn_info_delete, df_insn_rescan, and other
>
> df_insn_delete I suppose
Right.
> + df-scan routines that don't have to work in deferred mode and don't
> + have to mark basic blocks for re-processing. */
>
> -void
> -df_insn_delete (basic_block bb, unsigned int uid)
> +static void
> +df_insn_info_delete (unsigned int uid)
> {
>
> +/* Delete all of the refs information from INSN, either right now
> + or marked for later in deferred mode.
> +
> + FIXME: BB must be passed in, to mark the basic block containing
> + the insn as dirty, but emit-rtl.c doesn't do so. */
>
> you mean the remove_insn call? So it should call df_insn_info_delete instead?
Yes, remove_insn calls df_insn_delete with argument bb=NULL. This goes
against the interface documented in df-scan. It should call
df_insn_delete with a non-NULL bb argument.
df_insn_info_delete should remain internal to df-scan.c. (Ideally,
"struct df_insn_info" would be internal to DF, IMHO.)
> It _does_ mark the block as dirtly later, if it is not a debug insn.
Yes, it does. But it should have been left up to df_insn_delete to
make the block dirty. It looks like remove_insn only passes NULL
because it calls df_insn_delete on non-insns and it doesn't pass
BLOCK_FOR_INSN because it feeds BARRIERs to df_insn_delete -- totally
bogus of course...
For GCC 4.9 I want to make df_insn_delete take care of the basic block
argument itself, and remove all df_set_bb_dirty calls from emit-rtl.c.
> Your change changes behavior for df->changeable_flags &
> DF_DEFER_INSN_RESCAN btw,
Not really. All paths to df_insn_info_delete are non-deferred.
> so it needs someone DF aware to review
> and that makes it stage1 material as well I think.
Also good. I think the patch is quite safe but it's obviously not a
bug fix (except perhaps for making the dumps less confusing). Let's
consider this queued for GCC 4.9.
FWIW, I like to think of myself as quite DF aware ;-)
> It's also odd how insn_info == NULL is handled here.
Yes, for GCC 4.9 I want to put a gcc_assert in df_insn_info_delete to
make sure insn_info is always non-NULL. It can be NULL now for NOTEs
and BARRIERs (via the remove_insn path).
Ciao!
Steven
