[IRA] Analysis of register usage of functions for usage by IRA.

On 01-09-14 18:41, Ulrich Weigand wrote:
> Tom de Vries wrote:
>
>> 	* ira-costs.c (ira_tune_allocno_costs): Use
>> 	ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS to adjust costs.
>
> In debugging PR 53864 on s390x-linux, I ran into a weird change in behavior
> that occurs when the following part of this patch was checked in:
>
>> -	      if (ira_hard_reg_set_intersection_p (regno, mode, call_used_reg_set)
>> -		  || HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
>> -		cost += (ALLOCNO_CALL_FREQ (a)
>> -			 * (ira_memory_move_cost[mode][rclass][0]
>> -			    + ira_memory_move_cost[mode][rclass][1]));
>> +	      crossed_calls_clobber_regs
>> +		= &(ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (a));
>> +	      if (ira_hard_reg_set_intersection_p (regno, mode,
>> +						   *crossed_calls_clobber_regs))
>> +		{
>> +		  if (ira_hard_reg_set_intersection_p (regno, mode,
>> +						       call_used_reg_set)
>> +		      || HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
>> +		    cost += (ALLOCNO_CALL_FREQ (a)
>> +			     * (ira_memory_move_cost[mode][rclass][0]
>> +				+ ira_memory_move_cost[mode][rclass][1]));
>>   #ifdef IRA_HARD_REGNO_ADD_COST_MULTIPLIER
>> -	      cost += ((ira_memory_move_cost[mode][rclass][0]
>> -			+ ira_memory_move_cost[mode][rclass][1])
>> -		       * ALLOCNO_FREQ (a)
>> -		       * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
>> +		  cost += ((ira_memory_move_cost[mode][rclass][0]
>> +			    + ira_memory_move_cost[mode][rclass][1])
>> +			   * ALLOCNO_FREQ (a)
>> +			   * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
>>   #endif
>> +		}
>
> Before that patch, this code would penalize all call-clobbered registers
> (if the alloca is used across a call), and it would penalize *all* registers
> in a target-dependent way if IRA_HARD_REGNO_ADD_COST_MULTIPLIER is defined;
> the latter is completely independent of the presence of any calls.
>
> However, after that patch, the IRA_HARD_REGNO_ADD_COST_MULTIPLIER penalty
> is only applied for registers clobbered by calls in this function.  This
> seems a completely unrelated change, and looks just wrong to me ...
>
> Was this done intentionally or is this just an oversight?
>

Ulrich,

thanks for noticing this. I agree, this looks wrong, and is probably an 
oversight. [ It seems that s390 is the only target defining 
IRA_HARD_REGNO_ADD_COST_MULTIPLIER, so this problem didn't show up on any other 
target. ]

I think attached patch fixes it.

I've build the patch and ran the fuse-caller-save tests, and I'm currently 
bootstrapping and reg-testing it on x86_64.

Can you check whether this patches fixes the issue for s390 ?

Thanks,
- Tom

> Bye,
> Ulrich
>

On 03-09-14 18:58, Tom de Vries wrote:
> I've build the patch and ran the fuse-caller-save tests, and I'm currently
> bootstrapping and reg-testing it on x86_64.
>

Vladimir,

This patch fixes a problem (found on s390) in one of the committed 
fuse-caller-save patches. s390 is the only user of the 
IRA_HARD_REGNO_ADD_COST_MULTIPLIER target macro. The problem in the 
fuse-caller-save patch is that the code guarded by 
IRA_HARD_REGNO_ADD_COST_MULTIPLIER in ira_tune_allocno_costs is not 
call-related, but is now conditional on a ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS 
test. This patch fixes that.

Bootstrapped and reg-tested on x86_64. No issues found ( other than a 
non-reproducible failure while testing the non-bootstrap version: 
https://gcc.gnu.org/ml/gcc/2014-09/msg00065.html ).

OK for trunk ?

Thanks,
- Tom

2014-09-04  Tom de Vries  <tom@codesourcery.com>

	* ira-costs.c (ira_tune_allocno_costs): Don't conditionalize
	IRA_HARD_REGNO_ADD_COST_MULTIPLIER code on
	ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS.

>
>
> 0001-Fix-IRA_HARD_REGNO_ADD_COST_MULTIPLIER-in-ira_tune_a.patch
>
>
> diff --git a/gcc/ira-costs.c b/gcc/ira-costs.c
> index 774a958..57239f5 100644
> --- a/gcc/ira-costs.c
> +++ b/gcc/ira-costs.c
> @@ -2217,21 +2217,19 @@ ira_tune_allocno_costs (void)
>   	      crossed_calls_clobber_regs
>   		= &(ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (a));
>   	      if (ira_hard_reg_set_intersection_p (regno, mode,
> -						   *crossed_calls_clobber_regs))
> -		{
> -		  if (ira_hard_reg_set_intersection_p (regno, mode,
> +						   *crossed_calls_clobber_regs)
> +		  && (ira_hard_reg_set_intersection_p (regno, mode,
>   						       call_used_reg_set)
> -		      || HARD_REGNO_CALL_PART_CLOBBERED (regno, mode))
> -		    cost += (ALLOCNO_CALL_FREQ (a)
> -			     * (ira_memory_move_cost[mode][rclass][0]
> -				+ ira_memory_move_cost[mode][rclass][1]));
> +		      || HARD_REGNO_CALL_PART_CLOBBERED (regno, mode)))
> +		cost += (ALLOCNO_CALL_FREQ (a)
> +			 * (ira_memory_move_cost[mode][rclass][0]
> +			    + ira_memory_move_cost[mode][rclass][1]));
>   #ifdef IRA_HARD_REGNO_ADD_COST_MULTIPLIER
> -		  cost += ((ira_memory_move_cost[mode][rclass][0]
> -			    + ira_memory_move_cost[mode][rclass][1])
> -			   * ALLOCNO_FREQ (a)
> -			   * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
> +	      cost += ((ira_memory_move_cost[mode][rclass][0]
> +			+ ira_memory_move_cost[mode][rclass][1])
> +		       * ALLOCNO_FREQ (a)
> +		       * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
>   #endif
> -		}
>   	      if (INT_MAX - cost < reg_costs[j])
>   		reg_costs[j] = INT_MAX;
>   	      else

On 2014-09-04 3:37 AM, Tom de Vries wrote:
> On 03-09-14 18:58, Tom de Vries wrote:
>> I've build the patch and ran the fuse-caller-save tests, and I'm
>> currently
>> bootstrapping and reg-testing it on x86_64.
>>
>
> Vladimir,
>
> This patch fixes a problem (found on s390) in one of the committed
> fuse-caller-save patches. s390 is the only user of the
> IRA_HARD_REGNO_ADD_COST_MULTIPLIER target macro. The problem in the
> fuse-caller-save patch is that the code guarded by
> IRA_HARD_REGNO_ADD_COST_MULTIPLIER in ira_tune_allocno_costs is not
> call-related, but is now conditional on a
> ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS test. This patch fixes that.
>
> Bootstrapped and reg-tested on x86_64. No issues found ( other than a
> non-reproducible failure while testing the non-bootstrap version:
> https://gcc.gnu.org/ml/gcc/2014-09/msg00065.html ).
>
> OK for trunk ?
>

Yes, Tom.  Thanks for fixing the problem Ulrich found.