On Tue, May 14, 2013 at 11:04 PM, Jeff Law <l...@redhat.com> wrote: > On 05/15/2013 12:02 AM, Steven Bosscher wrote: >> >> On Wed, May 15, 2013 at 12:50 AM, Teresa Johnson wrote: >>> >>> On Tue, May 14, 2013 at 2:49 PM, Steven Bosscher wrote: >>>> >>>> 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? >> >> >> I'm not concerned about efficiency of the checker routines. They only >> run with checking enabled anyway. These checkers are almost the best >> "documentation" of the rules of GCC's intermediate representations >> that we have, so it's more important to me to make them easy to >> understand, and to make sure these verifiers themselves are complete >> and correct. > > Right. > > >> >> So another BB walk wouldn't be a problem IMHO. Some of the other >> checkers do far worse things (some of them include non-linear >> algorithms, for example). > > I've got no problem with another BB walk. I'll pre-approve the existing > patch with the checker moved back out.
Done, retested with bootstrap/regression on x86_64-unknown-linux-gnu, and committed as r198934. Steven - I was looking at rtl_verify_flow_info_1 last night and have some ideas about how to refactor it a bit. I will try to send a patch for that in the next day since I will be adding to that verification code with my other function splitting patches and it would be good to get that refactored first. Thanks, Teresa > > jeff -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413