Re: [PATCH] Fix lto bootstrap verification failure with -freorder-blocks-and-partition

[Teresa Johnson <tejohnson at google dot com>]

On Sat, Nov 16, 2013 at 12:33 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> When testing with -freorder-blocks-and-partition enabled, I hit a
>> verification failure in an LTO profiledbootstrap. Edge forwarding
>> performed when we went into cfg layout mode after bb reordering
>> (during compgotos) created a situation where a hot block was then
>> dominated by a cold block and was therefore remarked as cold. Because
>> bb reorder was complete at that point, it was not moved in the
>> physical layout, and we incorrectly went in and out of the cold
>> section multiple times.
>>
>> The following patch addresses that by fixing the layout when we move
>> blocks to the cold section after bb reordering is complete.
>>
>> Tested with an LTO profiledbootstrap with
>> -freorder-blocks-and-partition enabled. Ok for trunk?
>>
>> Thanks,
>> Teresa
>>
>> 2013-11-15  Teresa Johnson  <tejohnson@google.com>
>>
>>         * cfgrtl.c (fixup_partitions): Reorder blocks if necessary.
>
> computed_gotos just unfactors unified blocks that we use to avoid CFGs with
> O(n^2) edges. This is mostly to avoid problems with nonlinearity of other passes
> and to reduce the quadratic memory use case to one function at a time.
>
> I wonder if it won't be cleaner to simply unfactor those just before pass_reorder_blocks.
>
> Computed gotos are used e.g. in libjava interpreter to optimize the tight interpretting
> loop.  I think those cases would benefit from having at least scheduling/reordering
> and alignments done right.
>
> Of course it depends on how bad the compile time implications are (I think in addition
> to libjava, there was a lucier's testcase that made us to go for this trick) ,
> but I would prefer it over ading yet another hack into cfgrtl...
> We also may just avoid cfglayout cleanup_cfg while doing computed gotos...

Note I haven't done an extensive check to see if compgotos is the only
phase that goes back into cfglayout mode after bb reordering is done,
that's just the one that hit this. Eventually it might be good to
prevent going into cfglayout mode after bb reordering.

For now we could either fix up the layout as I am doing here. Or as
you suggest, prevent some cleanup/cfg optimization after bb reordering
is done. I thought about preventing the forwarding optimization after
bb reordering when splitting was on initially, but didn't want
enabling -freorder-blocks-and-partition to unnecessarily prevent
optimization. The reordering seemed reasonably straightforward so I
went with that solution in this patch.

Let me know if you'd rather have the solution of preventing the
forwarding (or maybe all all of try_optimize_cfg to be safe) under
-freorder-blocks-and-partition after bb reordering.

Thanks,
Teresa

>
> Honza



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413