Re: PATCH: Improve debuggability at -O0 and fix PR 29609

["Richard Guenther" <richard dot guenther at gmail dot com>]

On 9/4/07, Tristan Gingold <gingold@adacore.com> wrote:
>
> On Sep 4, 2007, at 11:35 AM, Richard Guenther wrote:
>
> > On 9/4/07, Tristan Gingold <gingold@adacore.com> wrote:
> >>
> >> On Sep 3, 2007, at 11:39 AM, Richard Guenther wrote:
> >>
> >>> I don't like this approach.  From a quick check I see that gimple
> >>> lowering retains
> >>> location information for both the if and the then branch
> >>> correctly in
> >>> lower_cond_expr
> >>> and the CFG created has the correct locus set on the edge already.
> >> Right.
> >>
> >>>   So where
> >>> is this information lost?
> >> Even if this location information were not lost, it couldn't be
> >> inserted without splitting the
> >> conditional jump (ie inverting the condition and add an unconditional
> >> jump for the other branch).
> >
> > Well, your example just had one goto - in this case we can improve the
> > situation without pessimizing the general case by setting the proper
> > line on the jump.  That is, the case where we have a single
> > conditional
> > jump with one fallthrough edge.
> >
> > So I ask you to improve this case first, set the correct location
> > separately
> > on the condition and the jump.  Like you did for
> > expand_gimple_basic_block
> Ok, will do.
>
> > (do you have a testcase for this fix?  that looks like an issue
> > that can be
> > fixed separately from the others).
> Yes, my original patch contains two testcases.  The second one should
> cover this case.
>
> >>>   I suppose it may be during expand where we call
> >>> do_jump via jumpif but the complete jump sequence has one
> >>> location.  So the
> >>> proper fix would probably to teach do_jump to take an alternate
> >>> location for the
> >>> jump.
> >> I can try to modify expand_gimple_cond_expr so that it splits the
> >> jump.  Note that every conditional jumps
> >> will be split (bad IMHO).
> >
> > I don't think so.  You only need to split jumps if none of the edges
> > is fallthrough
> > and both edges have a goto_locus set.  For example for
> >
> > void foobar(int p)
> > {
> >   if (p)
> >         foo();
> > }
> >
> > none of the edges have.
> Right.

I have found the thread at
http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01789.html
where you posted a patch for this and the discussion with Steven.  I
didn't see the
problem with the always executed conditional jump as well ;)  But
still, splitting the
block in cfgexpand is cleaner (conditional on the availability of a
goto_locus on the
edge).

Thanks,
Richard.