Re: [PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Dec 15, 2017 at 09:34:44AM +0100, Richard Biener wrote:
> Ugh...  the issue is that this difference might have many other
> -fcompare-debug issues, like when folding things?  Why is it a
> STATEMENT_LIST rather than a COMPOUND_EXPR?

I believe most of other foldings don't use TREE_SIDE_EFFECTS on whole
statements, just on expressions.  The possible exception is
STATEMENT_EXPRESSIONs.  As for COMPOUND_EXPR, you mean use it only if
we actually don't create a STATEMENT_LIST for other reasons?
Don't we optimize away COMPOUND_EXPR lhs if it doesn't have TREE_SIDE_EFFECTS,
and we'd need COMPOUND_EXPR to have no TREE_SIDE_EFFECTS as whole.

> >  Neither # DEBUG BEGIN_STMT nor that NOP_EXPR have
> > TREE_SIDE_EFFECTS, but STATEMENT_LIST has TREE_SIDE_EFFECTS already from
> > make_node and the gimplifier (and apparently the C++ FE too) checks
> > just that bit.  With { { { 0; } { 1; } { 2; } { 3; } } } one can end up
> > with quite large STATEMENT_LIST subtrees which in reality still don't
> > have any side-effects.
> > Maintaining accurate TREE_SIDE_EFFECTS bit on STATEMENT_LIST would be hard,
> > if we would only add and never remove statements, then we could just clear
> > it during make_node and set it whenever adding TREE_SIDE_EFFECTS statement
> > into it, but as soon as we sometimes remove from STATEMENT_LIST, or merge
> > STATEMENT_LISTs etc., maintaining this is too IMHO expensive, especially
> > when we usually just will not care about it.
> > So, I think it is better to just compute this lazily in the few spots where
> > we are interested about this, in real-world testcases most of the
> > STATEMENT_LISTs will have side-effects and should find them pretty early
> > when walking the tree.
> > As a side-effect, this patch will handle those
> > { { { 0; } { 1; } { 2; } { 3; } } } and similar then/else statement lists
> > better.
> 
> I don't like this too much.  Iff then we should do "real" lazy
> computation, like adding a TREE_SIDE_EFFECTS_VALID flag on STATEMENT_LIST,
> keeping TREE_SIDE_EFFECTS up-to-date when easily possible and when doing

How would that possible?  I have 3 nested STATEMENT_LISTs, and remove the
only statement with TREE_SIDE_EFFECTS from the innermost one.  I can clear
TREE_SIDE_EFFECTS_VALID from that STATEMENT_LIST easily, but what would fix
up the 2 parent ones?

> the expensive thing cache the result.  That said, I'm not convinced
> this will fix -fcompare-debug issues for good.  Is it really necessary
> to introduce this IL difference so early and in such an intrusive way?
> 
> Can't we avoid adding # DEBUG BEGIN_STMT when there's not already
> a STATEMENT_LIST around for example?

I'll defer that to Alex.  Or we could surely just unset TREE_SIDE_EFFECTS
when parsing a STATEMENT_LIST that contains just a single !TREE_SIDE_EFFECTS
statement other than the # DEBUG BEGIN_STMT markers.  The real question is
what we do without -g when removing stuff from the STATEMENT_LISTs.  Do we
fold those into the only statement if we end up with just one, or optimize
away completely if it contains none, or do we just keep around
STATEMENT_LIST containing just the 0; or nothing at all?
If that is the case and whether there is a STATEMENT_LIST or not depends
purely on whether we've ever created one, then perhaps clearing the
TREE_SIDE_EFFECTS during parsing, or starting with clear TREE_SIDE_EFFECTS
in make_node for STATEMENT_LIST and updating it on additions to the
STATEMENT_LIST would do the trick.

	Jakub