[Patch] 1/2 Expand sync_sub as sync_add if predicates don't match.

[David Daney]

David Daney wrote:
> Richard Sandiford wrote:
>> David Daney <ddaney@avtrex.com> writes:
>>  
>>> While implementing the atomic memory operations for MIPS, I came 
>>> across a problem where immediate operands were forced into a register 
>>> when sync_sub* insns were expanded.  On MIPS there is no minus 
>>> instruction for immediate operands, instead this should be expanded 
>>> as plus with the negative value of the immediate operand.
>>>
>>> In expand_sync_operation() and expand_sync_fetch_operation() there is 
>>> code to handle conversion to plus if the minus insn does not exits.  
>>> I enhanced this slightly so that it also does the conversion if the 
>>> predicate for sync_sub* does not match, but it does match for the 
>>> corresponding sync_add*.  This allows me to write sync_sub* insns 
>>> with a register predicate and have them used for non-immediate 
>>> operands, but an immediate operand results in conversion to sync_add*.
>>>
>>> Tested on x86_64-pc-linux-gnu all default languages both native and 
>>> -m32 with no regressions.  Also tested mips64-linux C only.
>>>
>>> :ADDPATCH middle-end:
>>>
>>> OK to commit?
>>>     
>>
>> FWIW, with (minus ... (const_int ...)) not being canonical rtl, I think
>> we should always use PLUS rather than MINUS if the operand is a 
>> CONST_INT.
>> E.g. the first thing expand_binop does is:
>>
>>   /* If subtracting an integer constant, convert this into an addition of
>>      the negated constant.  */
>>   if (binoptab == sub_optab && GET_CODE (op1) == CONST_INT)
>>     {
>>       op1 = negate_rtx (mode, op1);
>>       binoptab = add_optab;
>>     }
>>
>> and I think we should have a similar thing in expand_sync_operation
>> and expand_sync_fetch_operation.  As well as avoiding invalid
>> expressions, it has the advantage of folding the negation at
>> generation time, which I don't think the current version would.
>> (Sorry if that's wrong.)
>>   
> 
> The attached new revision of the patch does it this way now.  Light 
> testing on MIPS shows that it has the same effect as the first version.  
> Which from the point of view of my mips atomic memory operations patch 
> is sufficient.
> 
> I was a little hesitant to do it this way the first time because it has 
> the potential of adversely impacting a (hypothetical) port where
> 
> sync_sub<mode> is more efficient than sync_add<mode> the only place I 
> could think of where this would be plausible is for cases where negating 
> the value would cause it to no longer fit in the same number of bits as 
> the original value (-0x8000 on mips assuming there were a subiu 
> instruction).
> 
> If deemed better than the previous version, I will fully test this.  It 
> would however be nice to get feedback that this is really better before 
> exerting a lot more effort in testing.

Now tested on both i686-pc-linux-gnu and x86_64-pc-linux-gnu all default 
languages with no regressions found.

OK to commit?


> 
> 2007-08-21  David Daney  <ddaney@avtrex.com>
> 
>    * optabs.c (expand_sync_operation):  Use plus insn if minus
>    CONST_INT_P(val).
>    (expand_sync_fetch_operation):  Ditto.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: optabs.c
> ===================================================================
> --- optabs.c	(revision 127356)
> +++ optabs.c	(working copy)
> @@ -6444,7 +6444,7 @@ expand_sync_operation (rtx mem, rtx val,
>  
>      case MINUS:
>        icode = sync_sub_optab[mode];
> -      if (icode == CODE_FOR_nothing)
> +      if (icode == CODE_FOR_nothing || CONST_INT_P (val))
>  	{
>  	  icode = sync_add_optab[mode];
>  	  if (icode != CODE_FOR_nothing)
> @@ -6544,7 +6544,8 @@ expand_sync_fetch_operation (rtx mem, rt
>      case MINUS:
>        old_code = sync_old_sub_optab[mode];
>        new_code = sync_new_sub_optab[mode];
> -      if (old_code == CODE_FOR_nothing && new_code == CODE_FOR_nothing)
> +      if ((old_code == CODE_FOR_nothing && new_code == CODE_FOR_nothing)
> +          || CONST_INT_P (val))
>  	{
>  	  old_code = sync_old_add_optab[mode];
>  	  new_code = sync_new_add_optab[mode];