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?), but yours works too.

>> +  /* 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. 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, so it
seems unsuitable. So I've left that alone.

New version below.


Bernd