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