On Tue, May 14, 2013 at 2:49 PM, Steven Bosscher <stevenb....@gmail.com> wrote:
> On Tue, May 14, 2013 at 11:42 PM, Teresa Johnson wrote:
>>
>>         * function.h (has_bb_partition): New rtl_data flag.
>>         (bb_reorder_complete): Ditto.
>>         * cfgrtl.c (rtl_verify_flow_info_1): After bbro, verify
>>         that text sections switch at most once in layout order.
>>         * bb-reorder.c (connect_traces): Check for has_bb_partition
>>         instead of flag_reorder_blocks_and_partition.
>>         (verify_hot_cold_block_grouping): Remove.
>>         (reorder_basic_blocks): Remove call to deleted
>>         verify_hot_cold_block_grouping, and set bb_reorder_complete.
>>         (partition_hot_cold_basic_blocks): Set has_bb_partition.
>>         * cfgcleanup.c (try_crossjump_to_edge): Check for has_bb_partition
>>         instead of flag_reorder_blocks_and_partition.
>>
>
> I can't approve this (I'm probably the most experienced GCC
> contributor without any approval authority :-) but it looks good to
> me.
>
> One nit: Can you keep the verify_hot_cold_block_grouping function
> separate? rtl_verify_flow_info* is already too big and complex
> (somewhere down on my TODO list is splitting it up and improving
> cfglayout mode checking, e.g. to make sure there are no barriers/notes
> between basic blocks...).

Initially that's what I did, but then it seemed less efficient because
it adds another iteration over the BBs, so I instead merged the check
with one of the existing BB iterations in that routine. Do you still
prefer that I outline it?

As an alternative, perhaps rtl_verify_flow_info_1 could be split in
half in a follow-on patch since there are a couple walks over the BBs
in there. WDYT?

Thanks,
Teresa

>
> Ciao!
> Steven



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

Reply via email to