Re: [PATCH,RFC] CSE path following on basic blocks

[Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>]

Steven Bosscher wrote:
> On 11/28/06, Eric Botcazou <ebotcazou@libertysurf.fr> wrote:
> > !   /* There is a previous path.  Make sure it started with FIRST_BB.  */
> > !   if (path_size)
> > !     gcc_assert (data->path[0].bb = first_bb);
> >
> > Not sure what you're trying to do here.  Or maybe an annoying typo?
>
> If there is a path in data, then we're going to discover a new path
> starting with
> data->path[0].bb.  But Passing first_bb may be unnecessary.

But you have only one '=' above... :-)

> Yes.  Left-over from testing.  I sometimes ended up with path_size < 0
> due to a bug.  I'll remove this in the final patch submission.

Or assert path_size >= 0 nearer to the beginning of the functions.

> I'm not sure if we want to do this, though.  We could follow the
> fallthrough path even if flag_cse_follow_jumps is not set (which is
> indeed what CSE currently does). Or we could make CSE purely a local
> (i.e. intra basic block) pass if flag_cse_follow_jumps is not set.
> Personally, I don't think it matters much, but making it purely local
> is slightly easier and a lot more consistent.

FWIW, I agree with Steven.

> > *************** cse_insn (rtx insn, rtx libcall_insn)
> > *** 5647,5652 ****
> > --- 5652,5659 ----
> >    prev_insn_cc0_mode = this_insn_cc0_mode;
> >    prev_insn = insn;
> >  #endif
> > +
> > +   return;
> >  }
> >
> > Rather clever attempt to grow the size of the patch. :-)
>
> Actually, without this you get a warning about a label at the end of a
> compound statement on CC0-targets.  I found that out the hard way.

Then you could also replace "label:" with "label:;"

Paolo