This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] df-scan: split df_insn_delete for clearer dumps and better speed


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]