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

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Dec 18, 2017 at 02:02:52PM +0100, Jakub Jelinek wrote:
> Because make_node (STATEMENT_LIST) sets TREE_SIDE_EFFECTS, but the above
> memset clears it, we start with inconsistent TREE_SIDE_EFFECTS from the
> beginning.
> 
> I've tried to track TREE_SIDE_EFFECTS precisely, as below, but that seems to
> break completely the statement expression handling, in particular
> make check-gcc check-g++ RUNTESTFLAGS="compile.exp=20030530-3.c debug.exp='20020224-1.c 20030605-1.c' dg.exp='Wduplicated-branches-1.c compare2.c gnu89-const-expr-1.c pr64637.c pr68412-2.c stmt-expr-*.c pr56419.C' lto.exp=pr83388* noncompile.exp=971104-1.c dg-torture.exp=pr51071.c tree-ssa.exp='20030711-2.c 20030714-1.c 20030731-1.c' vect.exp=pr33833.c tm.exp=pr56419.C"
> all FAILs because of that, e.g.
> gcc/testsuite/gcc.c-torture/compile/20030530-3.c:14:8: error: void value not ignored as it ought to be
> I've also tried to track instead of tracking TREE_SIDE_EFFECTS precisely
> track just ignore DEBUG_BEGIN_STMTs in the processing, but that
> unfortunately doesn't help for the testcase included in the patch (attached
> patch).

One option would be to deal with that in pop_stmt_list and the C++ caller
thereof, where we right now have:

      /* If the statement list contained exactly one statement, then
         extract it immediately.  */
      if (tsi_one_before_end_p (i))
        {
          u = tsi_stmt (i);
          tsi_delink (&i);
          free_stmt_list (t);
          t = u;
        }

This part would need a MAY_HAVE_DEBUG_MARKER_STMTS variant that would
check for the case where there are just DEBUG_BEGIN_STMT markers + one other
stmt (+ maybe some further DEBUG_BEGIN_STMT markers) and nothing else.
If that one other stmt doesn't have TREE_SIDE_EFFECTS, clear
TREE_SIDE_EFFECTS on the whole statement list to get the same
TREE_SIDE_EFFECTS from the returned expression as without -g.

      /* If the statement list contained a debug begin stmt and a
         statement list, move the debug begin stmt into the statement
         list and return it.  */
      else if (!tsi_end_p (i)
               && TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
        {
          u = tsi_stmt (i);
          tsi_next (&i);
          if (tsi_one_before_end_p (i)
              && TREE_CODE (tsi_stmt (i)) == STATEMENT_LIST)
            {
              tree l = tsi_stmt (i);
              tsi_prev (&i);
              tsi_delink (&i);
              tsi_delink (&i);
              i = tsi_start (l);
              free_stmt_list (t);
              t = l;
              tsi_link_before (&i, u, TSI_SAME_STMT);
            }
        }

I don't understand how the above hack can work reliably, first of all,
it handles only a single DEBUG_BEGIN_STMT before, none after, and doesn't
recurse.  Perhaps it wouldn't be needed at all if we do the above, or would
be just an optimization for a common case, not a correctness fix?

	Jakub