On 12/03/2010 06:57 PM, Eric Botcazou wrote:
> 
> Almost OK, but:
> 
> +	  enum reg_class reg_class_superunion_in_chain = NO_REGS;
> 
> The name is too long, leading to
> 
> +	      reg_class_superunion_in_chain
> +		= reg_class_superunion[(int)reg_class_superunion_in_chain][(int)tmp->cl];
> 
> Just call it "superunion_class".
> 

OK, fixed.

> 
> +	  /* The register iteration order here is "preferred-register-first".
> +	     Firstly(i == 0), we iterate registers belong to class
> +	     PREFERRED_CLASS, if we find a new register, we stop immeidately.
> 
> "... we iterate over registers that belong to PREFERRED_CLASS; if we find a 
> new register, we stop immediately."
> 

Fixed.

> +	     Otherwise, we iterate once more (i == 1) registers, which
> +	     doesn't belong class PREFERRED_CLASS.
> 
> "...over registers that don't belong to PREFERRED_CLASS".
> 
> +	     If preferred class is not found by target hook, PREFERRED_CLASS
> +	     is NO_REGS, and registers are iterated in an ascending order
> +	     without any preference.  */
> 
> "If PREFERRED_CLASS is NO_REGS, we iterate over all registers in ascending 
> order without any preference".
> 

Fixed.

> But the loop is still run twice, isn't it?  Can we skip the first run if 
> PREFERRED_CLASS is NO_REGS, for example by starting at 1 instead of 0?
> 

Yes, loop still runs twice. Fixed like this,

for (pass = (preferred_class == NO_REGS ? 1 : 0); pass < 2; pass++)

> The usual convention for this kind of scheme (mutiple runs) in the RTL 
> optimizers is to use PASS as iteration variable:
> 
>   for (pass = 0; pass < 2; pass++)
> 
> and just test (pass == 0) or (pass == 1), i.e. don't use PREFER at all:
> 
>   /* In the first pass, iterate over registers first in preferred class.  */
>   if (pass == 0
>       && !TEST_HARD_REG_BIT (reg_class_contents[preferred_class], new_reg))
>     continue;
> 
> [...]
> 
>   /* If we find a new reg in our preferred class, stop immediately.  */
>   if (pass == 0 && best_new_reg != reg)
>     {
>       found = true;
>       break;
>     }
> 

PASS is a good variable name here! :)  Fixed.

> As for the hook itself: drop the PREFERRED_RENAME_CLASS macro entirely, i.e. 
> the default hook is just:
> 
> reg_class_t
> default_preferred_rename_class (reg_class_t rclass)
> {
>   return NO_REGS;
> }
> 
> and back-ends must implement the function (or use the default).
> 

Seems I didn't understand targethook fully before.  You are right.  Done.

-- 
Yao (齐尧)