Hi Haochen,

on 2024/7/11 13:50, HAO CHEN GUI wrote:
> Hi,
>   This patch adds TARGET_FLOAT128_HW into pattern conditions for quad-
> precision insns. Also it removes FLOAT128_IEEE_P check from pattern
> conditions if the mode of pattern is IEEE128 as the mode iterator -
> IEEE128 already checks with FLOAT128_IEEE_P.

I noticed that there are several patterns with similar useless 
FLOAT128_IBM_P condition, could you make a separated patch for both
redundant FLOAT128_IBM_P and FLOAT128_IEEE_P removal?  Then it can
be separated from this TARGET_FLOAT128_HW change and become purely
a NFC patch.

> 
>   For test case float128-cmp2-runnable.c, it should be guarded with
> ppc_float128_hw as it calls qp insns. The p9vector_hw is covered with
> ppc_float128_hw, so it's removed.
> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no
> regressions. Is it OK for trunk?
> 
> Thanks
> Gui Haochen
> 
> ChangeLog
> rs6000: Add TARGET_FLOAT128_HW guard for quad-precision insns
> 
> gcc/
>       * config/rs6000/rs6000.md (*fpmask<mode>, floatdidf2, floatti<mode>2,
>       floatunsti<mode>2, fix_trunc<mode>ti2): Add guard
>       TARGET_FLOAT128_HW.
>       (add<mode>3, sub<mode>3, mul<mode>3, div<mode>3, sqrt<mode>2,
>       copysign<mode>3_hard, copysign<mode>3_soft, @neg<mode>2_hw,
>       @abs<mode>2_hw, *nabs<mode>2_hw, fma<mode>4_hw, *fms<mode>4_hw,
>       *nfma<mode>4_hw, *nfms<mode>4_hw,
>       extend<SFDF:mode><IEEE128:mode>2_hw, trunc<mode>df2_hw,
>       trunc<mode>sf2_hw, fix<uns>_trunc<IEEE128:mode><QHI:mode>2,
>       *fix<uns>_trunc<IEEE128:mode><QHSI:mode>2_mem,
>       float_<mode>si2_hw, floatuns_<mode>di2_hw, floor<mode>2,
>       ceil<mode>2, btrunc<mode>2, round<mode>2, add<mode>3_odd,
>       sub<mode>3_odd, mul<mode>3_odd, div<mode>3_odd, sqrt<mode>2_odd,
>       fma<mode>4_odd, *fms<mode>4_odd, *nfma<mode>4_odd,
>       *nfms<mode>4_odd, trunc<mode>df2_odd, *cmp<mode>_hw for IEEE128):
>       Remove guard FLOAT128_IEEE_P.
>       * config/rs6000/vsx.md (xsxexpqp_<IEEE128:mode>_<V2DI_DI:mode>,
>       xsxsigqp_<IEEE128:mode>_<VEC_TI:mode>, xsiexpqpf_<mode>,
>       xsiexpqp_<IEEE128:mode>_<V2DI_DI:mode>, xscmpexpqp_<code>_<mode>,
>       *xscmpexpqp, xststdcnegqp_<mode>): Add guard TARGET_FLOAT128_HW.
>       (xststdc_<mode>, *xststdc_<mode>, xststdc_<mode>): Add guard
>       TARGET_FLOAT128_HW for the IEEE128 mode.
> 
> gcc/testsuite/
>       * testsuite/gcc.target/powerpc/float128-cmp2-runnable.c: Replace
>       ppc_float128_sw with ppc_float128_hw and remove p9vector_hw.
> 
> patch.diff

snip...

>     "xscmpuqp %0,%1,%2"
>    [(set_attr "type" "veccmp")
>     (set_attr "size" "128")])
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 56d1d8c737e..b5c143b1523 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -5157,7 +5157,7 @@ (define_insn "xsxexpqp_<IEEE128:mode>_<V2DI_DI:mode>"
>       (unspec:V2DI_DI
>         [(match_operand:IEEE128 1 "altivec_register_operand" "v")]
>        UNSPEC_VSX_SXEXPDP))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

TARGET_FLOAT128_HW checks ISA_3_0_MASKS_IEEE which has OPTION_MASK_P9_VECTOR,
so I think TARGET_P9_VECTOR is redundant here.

>    "xsxexpqp %0,%1"
>    [(set_attr "type" "vecmove")])
> 
> @@ -5176,7 +5176,7 @@ (define_insn "xsxsigqp_<IEEE128:mode>_<VEC_TI:mode>"
>       (unspec:VEC_TI [(match_operand:IEEE128 1
>                           "altivec_register_operand" "v")]
>        UNSPEC_VSX_SXSIG))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

>    "xsxsigqp %0,%1"
>    [(set_attr "type" "vecmove")])
> 
> @@ -5196,7 +5196,7 @@ (define_insn "xsiexpqpf_<mode>"
>        [(match_operand:IEEE128 1 "altivec_register_operand" "v")
>         (match_operand:DI 2 "altivec_register_operand" "v")]
>        UNSPEC_VSX_SIEXPQP))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

>    "xsiexpqp %0,%1,%2"
>    [(set_attr "type" "vecmove")])
> 
> @@ -5208,7 +5208,7 @@ (define_insn "xsiexpqp_<IEEE128:mode>_<V2DI_DI:mode>"
>                        (match_operand:V2DI_DI 2
>                         "altivec_register_operand" "v")]
>        UNSPEC_VSX_SIEXPQP))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

>    "xsiexpqp %0,%1,%2"
>    [(set_attr "type" "vecmove")])
> 
> @@ -5278,7 +5278,7 @@ (define_expand "xscmpexpqp_<code>_<mode>"
>     (set (match_operand:SI 0 "register_operand" "=r")
>       (CMP_TEST:SI (match_dup 3)
>                    (const_int 0)))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

>  {
>    if (<CODE> == UNORDERED && !HONOR_NANS (<MODE>mode))
>      {
> @@ -5296,7 +5296,7 @@ (define_insn "*xscmpexpqp"
>                         (match_operand:IEEE128 2 "altivec_register_operand" 
> "v")]
>         UNSPEC_VSX_SCMPEXPQP)
>        (match_operand:SI 3 "zero_constant" "j")))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

>    "xscmpexpqp %0,%1,%2"
>    [(set_attr "type" "fpcompare")])
> 
> @@ -5315,7 +5315,8 @@ (define_expand "xststdc_<mode>"
>     (set (match_operand:SI 0 "register_operand" "=r")
>       (eq:SI (match_dup 3)
>              (const_int 0)))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR
> +   && (!FLOAT128_IEEE_P (<MODE>mode) || TARGET_FLOAT128_HW)"
>  {
>    operands[3] = gen_reg_rtx (CCFPmode);
>    operands[4] = CONST0_RTX (SImode);
> @@ -5324,7 +5325,9 @@ (define_expand "xststdc_<mode>"
>  (define_expand "isinf<mode>2"
>    [(use (match_operand:SI 0 "gpc_reg_operand"))
>     (use (match_operand:IEEE_FP 1 "<vsx_altivec>"))]
> -  "TARGET_HARD_FLOAT && TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR
> +   && ((!FLOAT128_IEEE_P (<MODE>mode) && TARGET_HARD_FLOAT)
> +       || (FLOAT128_IEEE_P (<MODE>mode) && TARGET_FLOAT128_HW))"

This TARGET_HARD_FLOAT looks redundant, as !TARGET_HARD_FLOAT => !VSX
=> !TARGET_P9_VECTOR, then this can be simplified like the others:

  "TARGET_P9_VECTOR
   && (!FLOAT128_IEEE_P (<MODE>mode) || TARGET_FLOAT128_HW)"?

>  {
>    int mask = VSX_TEST_DATA_CLASS_POS_INF | VSX_TEST_DATA_CLASS_NEG_INF;
>    emit_insn (gen_xststdc_<mode> (operands[0], operands[1], GEN_INT (mask)));
> @@ -5343,7 +5346,7 @@ (define_expand "xststdcnegqp_<mode>"
>     (set (match_operand:SI 0 "register_operand" "=r")
>       (lt:SI (match_dup 2)
>              (const_int 0)))]
> -  "TARGET_P9_VECTOR"
> +  "TARGET_P9_VECTOR && TARGET_FLOAT128_HW"

Ditto.

BR,
Kewen

Reply via email to