Re: [Patch] Change to argument promotion in fixed conversion library calls

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Nov 6, 2015 at 8:14 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 11/06/2015 08:04 PM, Steve Ellcey  wrote:
>>
>> When I made this change I had one regression in the GCC testsuite
>> (gcc.dg/fixed-point/convert-sat.c).  I tracked this down to the
>> fact that emit_library_call_value_1 does not do any argument promotion
>> because it does not have the original tree type information for library
>> calls.  It only knows about modes.  I can't change
>> emit_library_call_value_1
>> to do the promotion because it does not know whether to do a signed or
>> unsigned promotion, but expand_fixed_convert could do the conversion
>> before calling emit_library_call_value_1 and that is what this patch does.
>
>
> Hmm, difficult. I can see how there would be a problem, but considering how
> many calls to emit_library_call_* we have, I'm slightly worried whether this
> is really is a good approach.
>
> On the other hand, there seems to be precedent in this file:
>
>       if (GET_MODE_PRECISION (GET_MODE (from)) < GET_MODE_PRECISION
> (SImode))
>         from = convert_to_mode (SImode, from, unsignedp);
>
>> The 'real' long term fix for this problem is to have tree types for
>> builtin
>> functions so the proper promotions can always be done but that is a fairly
>> large change that I am not willing to tackle right now and it could
>> probably
>> not be done in time for GCC 6.0 anyway.
>
>
> Yeah, but I agree that this is the real fix. We should aim to get rid of the
> emit_library_call functions.

Indeed.  In the "great plan" of simplifying RTL expansion by moving stuff
up to the GIMPLE level this could be done in a lowering stage lowering
all operations that we need to do via libcalls to GIMPLE calls.  Now,
we'd either need proper function declarations for all libcalls of optabs
for this or have the optab internal function stuff from Richard also provide
the libcall fallback.

In the expansion code for the as-libcall path we can then simply use the
type of the incoming argument (as we could if emit_library_call_value_1
would in addition to the RTX operands also receive the original tree ones).

Richard.


>> +  if (SCALAR_INT_MODE_P (from_mode))
>> +    {
>> +      /*  If we need to promote the integer function argument we need to
>> do
>
>
> Extra space at the start of the comment.
>
>> +         it here instead of inside emit_library_call_value because here
>> we
>> +         know if we should be doing a signed or unsigned promotion.  */
>> +
>> +      machine_mode arg_mode;
>> +      int unsigned_p = 0;
>> +
>> +      arg_mode = promote_function_mode (NULL_TREE, from_mode,
>> +                                       &unsigned_p, NULL_TREE, 0);
>> +      if (arg_mode != from_mode)
>> +       {
>> +         from = convert_to_mode (arg_mode, from, uintp);
>> +         from_mode = arg_mode;
>> +       }
>> +    }
>
>
> Move this into a separate function (prepare_libcall_arg)? I'll think about
> it over the weekend and let others chime in if they want, but I think I'll
> probably end up approving it with that change.
>
>
> Bernd