This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Jeff Law <law at redhat dot com>, Alexandre Oliva <aoliva at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 15 Dec 2017 09:50:38 +0100
- Subject: Re: [PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)
- Authentication-results: sourceware.org; auth=none
- References: <20171214200135.GY2353@tucnak> <alpine.LSU.2.20.1712150930570.12252@zhemvz.fhfr.qr>
- Reply-to: 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