[PATCH] BZ60501: Add addptr optab

[Andreas Krebbel]

On 13/03/14 12:25, Richard Biener wrote:
> On Thu, Mar 13, 2014 at 12:16 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> --- a/gcc/doc/md.texi
>>> +++ b/gcc/doc/md.texi
>>> @@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in
>>> operand 0.  All operands must have mode @var{m}.  This can be used even on
>>> two-address machines, by means of constraints requiring operands 1 and 0 to
>>> be the same location.
>>>
>>> +@cindex @code{addptr@var{m}3} instruction pattern
>>> +@item @samp{addptr@var{m}3}
>>> +Like @code{addptr@var{m}3} but does never clobber the condition code.
>>> +It only needs to be defined if @code{add@var{m}3} either sets the
>>> +condition code or address calculations cannot be performed with the
>>> +normal add instructions due to other reasons.  If adds used for
>>> +address calculations and normal adds are not compatible it is required
>>> +to expand a distinct pattern (e.g. using an unspec).  The pattern is
>>> +used by LRA to emit address calculations.  @code{add@var{m}3} is used
>>> +if @code{addptr@var{m}3} is not defined.
>>
>> I'm a bit skeptical of the "address calculations cannot be performed with the
>> normal add instructions due to other reasons" part".  Surely they can be
>> performed on all architectures supported by GCC as of this writing, otherwise
>> how would the compiler even work?  And if it's really like @code{add@var{m}3},
>> why restricting it to addresses, i.e. why calling it @code{addptr@var{m}3}?
>> Does that come from an implementation constraint on s390 that supports it only
>> for a subset of the cases supported by @code{add@var{m}3}?
> 
> Yeah, isn't it that you want a named pattern like add_nocc for an add
> that doesn't clobber flags?
This would suggest that you can use the pattern also for performing a normal add in case the
condition code is not needed afterwards but this isn't correct for s390 31 bit where an address
calculation is actually something different.  addptr is better I think because it is a pattern which
is supposed to be implemented with a load address instruction and the middle-end guarantees to use
it only on addresses. (I hope LRA is actually behaving that way). Perhaps loadptr or la or
loadaddress would be a better name?

Bye,

-Andreas-


> 
> Richard.
> 
>>> diff --git a/gcc/lra.c b/gcc/lra.c
>>> index 77074e2..e5e81474 100644
>>> --- a/gcc/lra.c
>>> +++ b/gcc/lra.c
>>> @@ -254,6 +254,19 @@ emit_add3_insn (rtx x, rtx y, rtx z)
>>>    rtx insn, last;
>>>
>>>    last = get_last_insn ();
>>> +
>>> +  if (have_addptr3_insn (x, y, z))
>>> +    {
>>> +      insn = gen_addptr3_insn (x, y, z);
>>> +
>>> +      /* If the target provides an "addptr" pattern it hopefully does
>>> +      for a reason.  So falling back to the normal add would be
>>> +      a bug.  */
>>> +      lra_assert (insn != NULL_RTX);
>>> +      emit_insn (insn);
>>> +      return insn;
>>> +    }
>>> +
>>>    insn = emit_insn (gen_rtx_SET (VOIDmode, x,
>>>                                gen_rtx_PLUS (GET_MODE (y), y, z)));
>>>    if (recog_memoized (insn) < 0)
>>
>> Same ambiguity here, emit_add3_insn is not documented as being restricted to
>> addresses:
>>
>> /* Emit insn x = y + z.  Return NULL if we failed to do it.
>>    Otherwise, return the insn.  We don't use gen_add3_insn as it might
>>    clobber CC.  */
>> static rtx
>> emit_add3_insn (rtx x, rtx y, rtx z)
>>
>> --
>> Eric Botcazou
>