RFA: patch to solve IRA PR48336, PR48342, PR48345

[Vladimir Makarov]

On 03/30/2011 06:19 AM, Richard Sandiford wrote:
> Hi Vlad,
>
> First, I want to echo H-P's thanks for tackling this area.  I just wondered:
>
> Vladimir Makarov<vmakarov@redhat.com>  writes:
>> The following patch is to solve PR48336, PR48342, PR48345.  The
>> profitable hard regs exclude hard regs which are prohibited for the
>> corresponding allocno mode. It is true for primary allocation and it is
>> important for better colorability criteria.  Function assign_hard_reg is
>> also based on this assumption.  Unfortunately, it is not true for
>> secondary allocation (after IRA IR flattening or during reload).  The
>> following patch solves this problem.
>>
>> The patch should be very safe but I am still testing it on x86/x86-64
>> bootstrap.
> [...]
>> Index: ira-color.c
>> ===================================================================
>> --- ira-color.c	(revision 171699)
>> +++ ira-color.c	(working copy)
>> @@ -1447,7 +1447,9 @@ update_conflict_hard_regno_costs (int *c
>>   }
>>
>>   /* Set up conflicting and profitable regs (through CONFLICT_REGS and
>> -   PROFITABLE_REGS) for each object of allocno A.  */
>> +   PROFITABLE_REGS) for each object of allocno A.  Remember that the
>> +   profitable regs exclude hard regs which can not hold value of mode
>> +   of allocno A.  */
>>   static inline void
>>   setup_conflict_profitable_regs (ira_allocno_t a, bool retry_p,
>>   				HARD_REG_SET *conflict_regs,
>> @@ -1463,8 +1465,13 @@ setup_conflict_profitable_regs (ira_allo
>>         COPY_HARD_REG_SET (conflict_regs[i],
>>   			 OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
>>         if (retry_p)
>> -	COPY_HARD_REG_SET (profitable_regs[i],
>> -			   reg_class_contents[ALLOCNO_CLASS (a)]);
>> +	{
>> +	  COPY_HARD_REG_SET (profitable_regs[i],
>> +			     reg_class_contents[ALLOCNO_CLASS (a)]);
>> +	  AND_COMPL_HARD_REG_SET (profitable_regs[i],
>> +				  ira_prohibited_class_mode_regs
>> +				  [ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)]);
>> +	}
>>         else
>>   	COPY_HARD_REG_SET (profitable_regs[i],
>>   			   OBJECT_COLOR_DATA (obj)->profitable_hard_regs);
> ira_prohibited_class_mode_regs is partly based on HARD_REGNO_MODE_OK,
> which is really a property of the first register in a multi-register
> group (rather than of every register in that group).  So is
> ira_prohibited_class_mode_regs defined in the same way?
> At the moment, check_hard_reg_p and setup_allocno_available_regs_num
> test profitability for every register in the group:
>
> 	/* Checking only profitable hard regs.  */
> 	if (TEST_HARD_REG_BIT (conflict_regs[k], hard_regno + j)
> 	    || ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno + j))
> 	  break;
> [...]
>        for (j = 0; j<  nregs; j++)
> 	{
> 	[...]
> 	      /* Checking only profitable hard regs.  */
> 	      if (TEST_HARD_REG_BIT (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
> 				     hard_regno + j)
> 		  || ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
> 					  hard_regno + j))
> 		break;
>
> So if you have a target in which double-word values have to start in
> even registers, I think every odd-numbered bit of profitable_hard_regs
> will be clear, and no register will seem profitable.  (I'm seeing this
> on ARM with some VFP tests.)
>
> Restricting the test to the first register fixes things for me,
> but do we need to check something else for the j != 0 case?
>
>
Richard, thanks very much for pointing this out.  I am agree with you.  
I think your patch is ok.  Although I need some time to check it.  Right 
now I am overwhelmed by # of other bug reports.

Another thing bothering me is a new colorability test for pseudos which 
should start on even or odd hard register.  I have suspicion that it is 
not ok for now.  I need some time to check and think about it.

I hope that I finish my urgent work on bug reports this week and start 
to work on your patch and the another issue on next week.

> gcc/
> 	* ira-color.c (check_hard_reg_p): Restrict the profitability check
> 	to the first register.
> 	(setup_allocno_available_regs_num): Likewise.
>
> Index: gcc/ira-color.c
> ===================================================================
> --- gcc/ira-color.c	(revision 171653)
> +++ gcc/ira-color.c	(working copy)
> @@ -1497,7 +1504,8 @@ check_hard_reg_p (ira_allocno_t a, int h
>         for (k = set_to_test_start; k<  set_to_test_end; k++)
>   	/* Checking only profitable hard regs.  */
>   	if (TEST_HARD_REG_BIT (conflict_regs[k], hard_regno + j)
> -	    || ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno + j))
> +	    || (j == 0
> +		&&  ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno)))
>   	  break;
>         if (k != set_to_test_end)
>   	break;
> @@ -2226,8 +2234,9 @@ setup_allocno_available_regs_num (ira_al
>   	      /* Checking only profitable hard regs.  */
>   	      if (TEST_HARD_REG_BIT (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
>   				     hard_regno + j)
> -		  || ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
> -					  hard_regno + j))
> +		  || (j == 0
> +		&&  ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
> +					      hard_regno)))
>   		break;
>   	    }
>   	  if (k != set_to_test_end)