Re: [PATCH] LoongArch: Add fcopysign instructions

[Xi Ruoyao <xry111@xry111.site>]

On Sun, 2022-11-06 at 09:57 +0800, Lulu Cheng wrote:
> 
> 在 2022/11/4 下午10:37, Xi Ruoyao 写道:
> > Add fcopysign.{s,d} with the names copysign{sf,df}3 so GCC will expand
> > __builtin_copysign{f,} to a single instruction.
> > 
> > Link: https://sourceware.org/pipermail/libc-alpha/2022-November/143177.html
> > 
> > gcc/ChangeLog:
> > 
> >         * config/loongarch/loongarch.md (UNSPEC_FCOPYSIGN): New unspec.
> >         (type): Add fcopysign.
> >         (copysign<mode>3): New instruction template.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >         * gcc.target/loongarch/fcopysign.c: New test.
> > ---
> > 
> > Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?
> > 
> >    gcc/config/loongarch/loongarch.md             | 22 ++++++++++++++++++-
> >    .../gcc.target/loongarch/fcopysign.c          | 16 ++++++++++++++
> >    2 files changed, 37 insertions(+), 1 deletion(-)
> >    create mode 100644 gcc/testsuite/gcc.target/loongarch/fcopysign.c
> > 
> > diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md
> > index 214b14bddd3..042e6e74491 100644
> > --- a/gcc/config/loongarch/loongarch.md
> > +++ b/gcc/config/loongarch/loongarch.md
> > @@ -37,6 +37,7 @@ (define_c_enum "unspec" [
> >      UNSPEC_FCLASS
> >      UNSPEC_FMAX
> >      UNSPEC_FMIN
> > +  UNSPEC_FCOPYSIGN
> >    
> >      ;; Override return address for exception handling.
> >      UNSPEC_EH_RETURN
> > @@ -214,6 +215,7 @@ (define_attr "qword_mode" "no,yes"
> >    ;; fabs              floating point absolute value
> >    ;; fneg              floating point negation
> >    ;; fcmp              floating point compare
> > +;; fcopysign   floating point copysign
> >    ;; fcvt              floating point convert
> >    ;; fsqrt     floating point square root
> >    ;; frsqrt       floating point reciprocal square root
> > @@ -226,7 +228,7 @@ (define_attr "type"
> >      "unknown,branch,jump,call,load,fpload,fpidxload,store,fpstore,fpidxstore,
> >       prefetch,prefetchx,condmove,mgtf,mftg,const,arith,logical,
> >       shift,slt,signext,clz,trap,imul,idiv,move,
> > -   fmove,fadd,fmul,fmadd,fdiv,frdiv,fabs,fneg,fcmp,fcvt,fsqrt,
> > +   fmove,fadd,fmul,fmadd,fdiv,frdiv,fabs,fneg,fcmp,fcopysign,fcvt,fsqrt,
> >       frsqrt,accext,accmod,multi,atomic,syncloop,nop,ghost"
> >      (cond [(eq_attr "jirl" "!unset") (const_string "call")
> >          (eq_attr "got" "load") (const_string "load")
> > @@ -976,6 +978,24 @@ (define_insn "abs<mode>2"
> >       (set_attr "mode" "<UNITMODE>")])
> >    \f
> >    ;;
> > +;;  ....................
> > +;;
> > +;;     FLOATING POINT COPYSIGN
> > +;;
> > +;;  ....................
> > +
> > +(define_insn "copysign<mode>3"
> > +  [(set (match_operand:ANYF 0 "register_operand" "=f")
> > +       (unspec:ANYF [(match_operand:ANYF 1 "register_operand" "f")
> > +                     (match_operand:ANYF 2 "register_operand" "f")]
> > +                    UNSPEC_FCOPYSIGN))]
> > +  ""
> 
> We need to add "TARGET_HARD_FLOAT" here.

It seems not strictly needed because ANYF expands to nothing if
!TARGET_HARD_FLOAT.  We also don't have an explicit TARGET_HARD_FLOAT in
fadd.{s,d} etc.

But I agree that adding it improves the readability.

> 
> Otherwise LGTM.
> 
> 
> Thanks!

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University