On Mon, Dec 20, 2021 at 3:24 AM Xionghu Luo <luo...@linux.ibm.com> wrote:
>
> These four UNSPECS seems could be replaced with native RTL, and why
> "(set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))"
> in the RTL pattern, per ISA of VSCR bit 127(VECTOR Saturation, SAT):
>
>   This bit is sticky; that is, once set to 1 it
>   remains set to 1 until it is set to 0 by an
>   mtvscr instruction.
>
> The RTL pattern set it to 0 but final ASM doesn't present it?  And why
> not use Clobber VSCR_REGNO instead?

The design came from the early implementation of Altivec:

https://gcc.gnu.org/pipermail/gcc-patches/2002-May/077409.html

If one later checks for saturation (reads VSCR), one needs a
corresponding SET of the value.  It's set in an architecture-specific
manner that isn't described to GCC, but it's set, not just clobbered
and in an undefined state.

The RTL does not describe that VSCR is set to the value 0.  The
(const_int 0) is not the value set.  You can think of the (const_int
0) as a dummy RTL argument to the VSCR UNSPEC.  UNSPEC requires at
least one argument and the pattern doesn't try to express the
argument, so it uses a dummy RTL constant.  It's part of a PARALLEL
and the plus or minus already expresses the data dependency of the
pattern on the input operands.

I'm unsure of the meaning of your question "final ASM doesn't present
it".  The operation on VSCR is implicit and not emitted as an
instruction.  It's in a PARALLEL, which means that the single Altivec
instruction has both effects.  Is that what you were asking?

> Tested pass on P10, OK for master?

This patch is okay.  Thanks for updating the machine description and
for cleaning up the formatting.

> Thanks.
>
> gcc/ChangeLog:
>
>         * config/rs6000/altivec.md (altivec_vaddu<VI_char>s): Replace
>         UNSPEC_VADDU with us_plus.
>         (altivec_vadds<VI_char>s): Replace UNSPEC_VADDS with ss_plus.
>         (altivec_vsubu<VI_char>s): Replace UNSPEC_VSUBU with us_minus.
>         (altivec_vsubs<VI_char>s): Replace UNSPEC_VSUBS with ss_minus.
>         (altivec_abss_<mode>): Likewise.
> ---
>  gcc/config/rs6000/altivec.md | 29 ++++++++++-------------------
>  1 file changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
> index a057218aa28..b2909857c34 100644
> --- a/gcc/config/rs6000/altivec.md
> +++ b/gcc/config/rs6000/altivec.md
> @@ -29,8 +29,6 @@ (define_c_enum "unspec"
>     UNSPEC_VMHADDSHS
>     UNSPEC_VMHRADDSHS
>     UNSPEC_VADDCUW
> -   UNSPEC_VADDU
> -   UNSPEC_VADDS
>     UNSPEC_VAVGU
>     UNSPEC_VAVGS
>     UNSPEC_VMULEUB
> @@ -61,8 +59,6 @@ (define_c_enum "unspec"
>     UNSPEC_VSR
>     UNSPEC_VSRO
>     UNSPEC_VSUBCUW
> -   UNSPEC_VSUBU
> -   UNSPEC_VSUBS
>     UNSPEC_VSUM4UBS
>     UNSPEC_VSUM4S
>     UNSPEC_VSUM2SWS
> @@ -517,9 +513,8 @@ (define_insn "altivec_vaddcuw"
>
>  (define_insn "altivec_vaddu<VI_char>s"
>    [(set (match_operand:VI 0 "register_operand" "=v")
> -        (unspec:VI [(match_operand:VI 1 "register_operand" "v")
> -                   (match_operand:VI 2 "register_operand" "v")]
> -                  UNSPEC_VADDU))
> +        (us_plus:VI (match_operand:VI 1 "register_operand" "v")
> +                   (match_operand:VI 2 "register_operand" "v")))
>     (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
>    "<VI_unit>"
>    "vaddu<VI_char>s %0,%1,%2"
> @@ -527,9 +522,8 @@ (define_insn "altivec_vaddu<VI_char>s"
>
>  (define_insn "altivec_vadds<VI_char>s"
>    [(set (match_operand:VI 0 "register_operand" "=v")
> -        (unspec:VI [(match_operand:VI 1 "register_operand" "v")
> -                    (match_operand:VI 2 "register_operand" "v")]
> -                  UNSPEC_VADDS))
> +        (ss_plus:VI (match_operand:VI 1 "register_operand" "v")
> +                   (match_operand:VI 2 "register_operand" "v")))
>     (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
>    "VECTOR_UNIT_ALTIVEC_P (<MODE>mode)"
>    "vadds<VI_char>s %0,%1,%2"
> @@ -563,9 +557,8 @@ (define_insn "altivec_vsubcuw"
>
>  (define_insn "altivec_vsubu<VI_char>s"
>    [(set (match_operand:VI 0 "register_operand" "=v")
> -        (unspec:VI [(match_operand:VI 1 "register_operand" "v")
> -                    (match_operand:VI 2 "register_operand" "v")]
> -                  UNSPEC_VSUBU))
> +        (us_minus:VI (match_operand:VI 1 "register_operand" "v")
> +                    (match_operand:VI 2 "register_operand" "v")))
>     (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
>    "VECTOR_UNIT_ALTIVEC_P (<MODE>mode)"
>    "vsubu<VI_char>s %0,%1,%2"
> @@ -573,9 +566,8 @@ (define_insn "altivec_vsubu<VI_char>s"
>
>  (define_insn "altivec_vsubs<VI_char>s"
>    [(set (match_operand:VI 0 "register_operand" "=v")
> -        (unspec:VI [(match_operand:VI 1 "register_operand" "v")
> -                    (match_operand:VI 2 "register_operand" "v")]
> -                  UNSPEC_VSUBS))
> +        (ss_minus:VI (match_operand:VI 1 "register_operand" "v")
> +                    (match_operand:VI 2 "register_operand" "v")))
>     (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
>    "VECTOR_UNIT_ALTIVEC_P (<MODE>mode)"
>    "vsubs<VI_char>s %0,%1,%2"
> @@ -3480,9 +3472,8 @@ (define_expand "altivec_absv4sf2"
>  (define_expand "altivec_abss_<mode>"
>    [(set (match_dup 2) (vec_duplicate:VI (const_int 0)))
>     (parallel [(set (match_dup 3)
> -                  (unspec:VI [(match_dup 2)
> -                              (match_operand:VI 1 "register_operand" "v")]
> -                             UNSPEC_VSUBS))
> +                  (ss_minus:VI (match_dup 2)
> +                               (match_operand:VI 1 "register_operand" "v")))
>               (set (reg:SI VSCR_REGNO)
>                    (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))])
>     (set (match_operand:VI 0 "register_operand" "=v")
> --
> 2.27.0
>

Reply via email to