Rename across basic block boundaries

[Richard Sandiford]

Bernd Schmidt <bernds@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