On Wed, Nov 18, 2020 at 08:31:28AM +0100, 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).
We can handle the constants issue similarly to what we do for
__builtin_fpclassify, too.
Segher
