Bernd Schmidt <ber...@codesourcery.com> writes: > On 08/26/11 14:57, Richard Sandiford wrote: >>> + /* There must be some kind of conflict. Set the unusable for all >>> + overlapping registers. */ >>> + min_reg = chain->regno; >>> + if (incoming_nregs < 0) >>> + min_reg += incoming_nregs; >>> + max_reg = chain->regno + chain->nregs; >>> + for (i = min_reg; i < max_reg; i++) >>> + ri->incoming[i].unusable = true; >> >> In the incoming_nregs < 0 case, we only need to set >> ri->incoming[chain->regno + incoming_nregs] itself, right, >> not the other registers between that and ri->incoming[chain->regno]? >> If so, I think it'd be clearer to have: >> >> /* There must be some kind of conflict. Prevent both the old and >> new ranges from being used. */ >> if (incoming_nregs < 0) >> ri->incoming[chain->regno + incoming_nregs].unusable = true; >> for (i = 0; i < chain->nregs; i++) >> ri->incoming[chain->regno + i].unusable = true; >> >> When I first looked at the code, I was wondering why we changed every >> register in (chain->regno + incoming_nregs, chain_regno), but none in >> [chain->regno + chain->nregs, OLD_END). Seems like we should do neither >> (as in the above suggestion) or both. > > I think both was the original intention (OLD_END being min_reg + > ri->incoming[min_reg].nregs, right?),
Right. There'd have been a max operation on max_reg too. > but yours works too. Ok, great. >>> + /* Process all basic blocks by using a worklist, adding unvisited >>> successor >>> + blocks whenever we reach the end of one basic blocks. This ensures >>> that >>> + whenever possible, we only process a block after at least one of its >>> + predecessors, which provides a "seeding" effect to make the logic in >>> + set_incoming_from_chain and init_rename_info useful. */ >> >> Wouldn't a reverse post-order (inverted_post_order_compute) allow even >> more pre-opening (as well as being less code)? > > I can't really tell from the comments what that function is supposed to > produce. Sorry, I thought it was supposed to produce a reverse postorder, but... > I've made a change to use it to order the bbs, but that made rr > visit basic blocks without seeing any of their predecessors first, ...I guess not. :-) pre_and_rev_post_order_compute should though. Could you try that instead? Richard
