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