Re: [PATCH] Sink clobbers if EH block contains just clobbers (PR tree-optimization/51117)

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Dec 12, 2011 at 03:46:01PM +0100, Michael Matz wrote:
> > I had to tweak a little bit the expander conflict checking, because
> > if we have a BB with two incoming EH edges and clobber stmts from both
> > sunk into its beginning, then it would consider both variables (a and b
> > above) to be live at the same time, while there is no insn on which they
> > can actually live at the same time, the PHIs don't mention either of them
> > (and after all, PHIs aren't memory loads), and after the PHIs we have
> > immediately the clobbers.
> 
> The idea is sound, the implementation can be tidied with the observation 
> that only the first real instruction (instead of the BB start) is the 
> point at which all currently live things need to be conflicted, like in 
> the patch below (only cfgexpand.c part changed).  I.e. moving the existing 
> code from add_scope_clobbers_1 a bit is enough.  I'm putting this through 
> regstrapping on x86_64-linux and will commit if that succeeds, given rths 
> approval for the other parts.

Looks cleaner, yes.
Just I wonder:
1) what if a bb contains no real insns (I know, they should be optimized
   out, but do we assert that etc.?) - then the EXECUTE_IF_SET_IN_BITMAP
   loop just wouldn't be done.  Perhaps that is fine, it would make it
   into the bitmap at the end of the bb and perhaps following bb would
   do this loop.
2) the PHIs are then handled always with visit_op instead of visit_conflict,
   I'd guess the needed add_stack_var_conflict calls would then happen
   in that EXECUTE_IF_SET_IN_BITMAP loop, right?

> I wonder how to best test this.

One kind of testing was watching the size of .gcc_except_table going down
with each patch (vanilla -> optimize_cloobers -> optimize_clobbers thinko
fix -> sink_clobbers).

The following test unfortunately succeeds already with current trunk
(i.e. tests already the optimize_clobbers optimization), and with
throw; instead of return 1; it contains __cxa_rethrow call even
with the sink optimization and the difference is mainly that
.gcc_except_table is smaller and __cxa_rethrow block is reachable just from
one spot (recorded in .gcc_except_table), while without sink_clobbers
there is also a jump to it from another spot.  For the
s/return 1/throw/ testcase we could perhaps
scan-tree-dump-not the optimized dump for __builtin_eh_copy_values.

// { dg-do compile }
// { dg-options "-O2 -fexceptions" }

struct A { char buf[64]; };
void bar (A *);

int
foo ()
{
  A c;
  bar (&c);
  try
  {
    {
      A a;
      bar (&a);
      if (a.buf[13])
	throw 1;
      else if (a.buf[52])
	throw 3;
    }
    {
      A b;
      bar (&b);
      if (b.buf[13])
	throw 2;
    }
  }
  catch ( ...)
  {
    return 1;
  }
  return 0;
}

// { dg-final { scan-assembler-not "__cxa_rethrow" } }


	Jakub