Hi!
On Mon, Jun 29, 2020 at 08:49:05AM +0200, Richard Biener via Gcc-patches wrote:
> On Fri, Jun 26, 2020 at 10:12 PM Raoni Fassina Firmino via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> > This is an early draft I'm working on to add fegetround , feclearexcept
> > and feraiseexcept as builtins on rs6000. This is my first patch so I
> > welcome any and all feedback. Foremost I have some questions to ask as
> > I got stuck on some problems.
> > Q2) How to fallback to the default behavior of the function call when
> > the builtin is not suitable for the parameters?
In general, in the expander you can do FAIL in such cases. For some
patterns that isn't allowed, and you have to copy everything the
standard implemntation does to your specialized implementation. This
of course doesn't scale, and will make you miss all future changes to
the standard implementation. It is then probably better to then do the
work the original implementatyion skimped on, and *do* allow FAIL.
> > Here, it is more specifically for feclearexcept and feraiseexcept. The
> > builtin should only be used in the case of the parameter input is a
> > constant number with only 1bit mask (to work on only one exception).
rs6000 has exact_log2_cint_operand (and the "N" constraint).
> > Q3) Are the implementations for the builtins more or less on the
> > right places?
> >
> > The first one I did was fegetround and I based it on ppc_get_timebase
> > and other related builtins, so I used a define_expand on rs6000.md, but
> > when I was working on the fe*except I was basing it on other builtins
> > and ended up implementing it all on rs6000-call.c, but I am not sure if
> > there is a canonical way of doing it one way or another.
Patterns have to go to one of the .md files. rs6000.md is a fine choice;
but put this somewhere near the other floating point patterns?
> GCC already knows fe* builtins, what GCC does not yet have is
> a way for targets to specify custom expansion of them. So instead
> of adding powerpc specific builtins you should add optabs for the
> RTL expansion part.
Yes.
> > +static rtx
> > +rs6000_expand_feCRexcept_builtin (enum insn_code icode, tree exp, rtx
> > target)
No caMel case please. "fe" and "cr" do not mean to much here; think of
a nicer name please? Saving a character or two isn't useful, this is
called in only a few places.
> > + //|| INTVAL (op0) == FE_INVALID)
Please fix.
> > +;; int __builtin_fegetround()
> > +(define_expand "rs6000_fegetround"
> > + [(use (match_operand:SI 0 "gpc_reg_operand"))]
> > + "TARGET_HARD_FLOAT"
> > +{
> > + rtx tmp_df = gen_reg_rtx (DFmode);
> > + emit_insn (gen_rs6000_mffsl (tmp_df));
> > +
> > + rtx tmp_di = simplify_gen_subreg (DImode, tmp_df, DFmode, 0);
> > + rtx tmp_di_2 = gen_reg_rtx (DImode);
> > + emit_insn (gen_anddi3 (tmp_di_2, tmp_di, GEN_INT (0x3LL)));
Just GEN_INT (3) will do fine.
Another question. How do these builtins prevent other FP insns from
being moved (or optimised) "over" them?
Segher
