Re: ifcvt/crossjump patch: Fix PR 42496, 21803

[Paolo Bonzini <bonzini at gnu dot org>]

On 07/29/2010 05:26 PM, Jeff Law wrote:
> > >
> > All I can see in cfgcleanup or cfgrtl are manual calls to
> > df_set_bb_dirty to show we don't know anything about register lives in
> > modified blocks.  This is also done by my new pass if it modifies stuff.
>
> So we've got the markers to allow us to do a deferred update, but with
> nothing needing the accurate DF info in the cfgcleanup loop, we just
> ignore the markers (within the context of that loop).    Which certainly
> makes sense if nothing in cfgcleanup has needed accurate register
> lifetime until now.

Correct.  _Operand scan_ can be made automatic, but not dataflow analysis.

> I'm having an awful hard time seeing what modeling this optimization as
> a series of CFG manipulations is going to win us right now.   It's more
> complex than using reorder_insns, it doesn't keep the DF information
> up-to-date, and generates unnecessary churn in the CFG and other data
> structures.

Just to state it once more, I agree.

I have two remaining doubts, which were shadowed by the discussion so 
far.  Don't worry, it's small stuff. :)

I'd like to have a note to the reader that df_analyze is only invoked 
when you do crossjumping.  Please add an assert like

  if (block_was_dirty)
    {
      gcc_assert (mode & CLEANUP_CROSSJUMP);
      df_analyze ();
    }

We do not use dataflow otherwise, and it is not necessary to call it 
gratuitously.  Passes know that CFG cleanup destroys dataflow and call 
it themselves if necessary.

Second, crossjumping is now more expensive.  Does it buy much really to 
iterate it?  Something like

  mode &= ~CLEANUP_CROSSJUMP;

just before iterating may still leave it "good enough".  Steven, do you 
remember anything?  This anyway can be done separately after the patch 
goes in.

Paolo