Re: RFC: LRA for x86/x86-64 [7/9] -- continuation

[Richard Sandiford <rdsandiford@googlemail.com>]

Sorry, reading back in different surroundings made me notice a couple
of silly errors:

Richard Sandiford <rdsandiford@googlemail.com> writes:
> E.g.:
>
>   if ((*loc = get_equiv_substitution (reg)) != reg)
>     ...as above...
>   if (*loc != reg || !in_class_p (reg, cl, &new_class))
>     ...as above...
>   else if (new_class != NO_REGS && rclass != new_class)
>     change_class (regno, new_class, "	   Change", true);
>   return change_p;
>
> (assuming change to in_class_p suggested earlier) seems like it
> covers the same cases.

...but that same in_class_p change means that the "rclass != new_class"
condition isn't needed.  I.e.

  if ((*loc = get_equiv_substitution (reg)) != reg)
    ...as above...
  if (*loc != reg || !in_class_p (reg, cl, &new_class))
    ...as above...
  else if (new_class != NO_REGS)
    change_class (regno, new_class, "	   Change", true);
  return change_p;

>> +	      if (operand_reg[nop] != NULL_RTX)
>> +		{
>> +		  int last_reload = (lra_reg_info[ORIGINAL_REGNO
>> +						  (operand_reg[nop])]
>> +				     .last_reload);
>> +
>> +		  if (last_reload > bb_reload_num)
>> +		    reload_sum += last_reload;
>> +		  else
>> +		    reload_sum += bb_reload_num;
>
> The comment for reload_sum says:
>
>> +/* Overall number reflecting distances of previous reloading the same
>> +   value.  It is used to improve inheritance chances.  */
>> +static int best_reload_sum;
>
> which made me think of distance from the current instruction.  I see
> it's actually something else, effectively a sum of instruction numbers.
>
> I assumed the idea was to prefer registers that were reloaded more
> recently (closer the current instruction).  In that case I thought that,
> for a distance-based best_reload_sum, smaller would be better,
> while for an instruction-number-based best_reload_sum, larger would
> be better.  It looks like we use instruction-number based best_reload_sums
> but prefer smaller sums:
>
>> +			      && (reload_nregs < best_reload_nregs
>> +				  || (reload_nregs == best_reload_nregs
>> +				      && best_reload_sum < reload_sum))))))))
>
> Is that intentional?

Clearly I can't read.  The code _does_ prefer higher numbers.  I still
think "distance" is a bit misleading though. :-)

Just for the record, the rest of my question:

> Also, is this value meaningful for output reloads, which aren't really
> going to be able to inherit a value as such?  We seem to apply the cost
> regardless of whether it's an input or an output, so probably deserves
> a comment.
>
> Same for matched input operands, which as you say elsewhere aren't
> inherited.

still applies.

Richard