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).
Richard.
