[PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Jeff Law]

On 11/18/20 12:31 AM, Richard Biener wrote:
> On Tue, 17 Nov 2020, Jeff Law wrote:
>
>>
>> On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
>>> On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
>>>>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
>>>>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
>>>>> +static rtx
>>>>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
>>>>> +{
>>>>> +  if (!validate_arglist (exp, VOID_TYPE))
>>>>> +    return NULL_RTX;
>>>>> +
>>>>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
>>>>> +  if (icode == CODE_FOR_nothing)
>>>>> +    return NULL_RTX;
>>>>> +
>>>>> +  if (target == 0
>>>>> +      || GET_MODE (target) != target_mode
>>>>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
>>>>> +    target = gen_reg_rtx (target_mode);
>>>>> +
>>>>> +  rtx pat = GEN_FCN (icode) (target);
>>>>> +  if (!pat)
>>>>> +    return NULL_RTX;
>>>>> +  emit_insn (pat);
>>>> I think you need to verify whether the expansion ended up in 'target'
>>>> and otherwise emit a move since usually 'target' is just a hint.
>>> I thought the "if (target == 0 ..." took care of that. The expands do
>>> emit a move, if that helps.
>> It looks like if we have a passed in target and it either has the wrong
>> mode or it does not match the predicate, then we generaet a new target
>> and use that instead.? I don't see where we'd copy from that new target
>> to the original desired target.? For some expanders the caller would
>> handle that, but I don't see how that's possible for this one without
>> the caller digging into the generated RTL to determine that
>> expand_builtin_fegetround put the result somewhere other than TARGET and
>> thus a copy is needed.
>>
>> That may be what Richi is worried about.
> I know we've added missing
>
>   if (!rtx_equal_p (target, ops[0].value))
>     emit_move_insn (target, ops[0].value);
>
> to several expanders (using expand_insn rather than manual
> GEN_FCN (icode) calls).
Yes.  But I think we end up doing that mostly for expanders that return
the object where the value was stored in some reasonably convenient
location (either as a return value or in an ops array).  I don't think
that's the case here. 

Jeff