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.
For feclearexcept and feraiseexcept I included tests to variable
'target', including none, but now I see that I did not do the same for
fegetround, I can add the same if it is necessary, but the test do check
if the return is correct, so I don't know.
> > +@cindex @code{fegetround@var{m}} instruction pattern
> > +@item @samp{fegetround@var{m}}
> > +Store the current machine floating-point rounding mode into operand 0.
> > +Operand 0 has mode @var{m}, which is scalar. This pattern is used to
> > +implement the @code{fegetround} function from the ISO C99 standard.
>
> I think this needs to elaborate on the format of the "rounding mode".
>
> AFAICS you do nothing to marshall with the actually used libc
> implementation which AFAIU can choose arbitrary values for
> the FE_* macros. I'm not sure we require the compiler to be
> configured for one specific C library and for example require
> matching FE_* macro definitions for all uses of the built
> compiler.
>
> For the patch at hand you seem to assume the libc "format"
> matches the hardware one (which would of course be reasonable).
>
> Does that actually hold up when looking at libcs other than
> glibc supporting powerpc?
I checked in some other libc implementations that have POWER support and
all have the same value as glic for the four rounding modes and the five
exception flags from libc. The libcs implementations I checked are:
- musl
- uclibc & uclibc-ng
- freebsd
Is There any other I am missing?
> If all of these are non-issues then the middle-end pices look OK.
> If we need any such "translation" layer then I guess we need
> to either have additional operands to the optabs specifying
> all of the FE_* values relevant for the respective call or
> provide a side-channel (target hook) to implement the
> translation on the expansion side.
IMHO, It seems like it is not necessary if there not a libc that have
different values for the FE_* macros. I didn't check other archs, but if
is the case for some other arch I think it could be changed if and when
some other arch implements expands for these builtins.
o/
Raoni
