rs6000: Make 32 bit stack_protect support prefixed insn [PR111367]

  Hi,

As PR111367 shows, with prefixed insn supported, some of
checkings consider it's able to leverage prefixed insn
for stack protect related load/store, but since we don't
actually change the emitted assembly for 32 bit, it can
cause the assembler error as exposed.

Mike's commit r10-4547-gce6a6c007e5a98 has already handled
the 64 bit case (DImode), this patch is to treat the 32
bit case (SImode) by making use of mode iterator P and
ptrload attribute iterator, also fixes the constraints
to match the emitted operand formats.

Bootstrapped and regtested on powerpc64-linux-gnu P7/P8/P9
and powerpc64le-linux-gnu P9.

This patch has incorporated Segher's comments in PR111367,
I'm going to push this soon if no objections.

BR,
Kewen
-----
	PR target/111367

gcc/ChangeLog:

	* config/rs6000/rs6000.md (stack_protect_setsi): Support prefixed
	instruction emission and incorporate to stack_protect_set<mode>.
	(stack_protect_setdi): Rename to ...
	(stack_protect_set<mode>): ... this, adjust constraint.
	(stack_protect_testsi): Support prefixed instruction emission and
	incorporate to stack_protect_test<mode>.
	(stack_protect_testdi): Rename to ...
	(stack_protect_test<mode>): ... this, adjust constraint.

gcc/testsuite/ChangeLog:

	* g++.target/powerpc/pr111367.C: New test.
---
 gcc/config/rs6000/rs6000.md                 | 73 ++++++++-------------
 gcc/testsuite/g++.target/powerpc/pr111367.C | 22 +++++++
 2 files changed, 49 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/pr111367.C

--
2.40.1
  

On Wed, Sep 27, 2023 at 1:38 AM Kewen.Lin <linkw@linux.ibm.com> wrote:

> Hi,
>
> As PR111367 shows, with prefixed insn supported, some of
> checkings consider it's able to leverage prefixed insn
> for stack protect related load/store, but since we don't
> actually change the emitted assembly for 32 bit, it can
> cause the assembler error as exposed.
>
> Mike's commit r10-4547-gce6a6c007e5a98 has already handled
> the 64 bit case (DImode), this patch is to treat the 32
> bit case (SImode) by making use of mode iterator P and
> ptrload attribute iterator, also fixes the constraints
> to match the emitted operand formats.
>
> Bootstrapped and regtested on powerpc64-linux-gnu P7/P8/P9
> and powerpc64le-linux-gnu P9.
>
> This patch has incorporated Segher's comments in PR111367,
> I'm going to push this soon if no objections.
>

This patch is okay.

Thanks, David


>
> BR,
> Kewen
> -----
>         PR target/111367
>
> gcc/ChangeLog:
>
>         * config/rs6000/rs6000.md (stack_protect_setsi): Support prefixed
>         instruction emission and incorporate to stack_protect_set<mode>.
>         (stack_protect_setdi): Rename to ...
>         (stack_protect_set<mode>): ... this, adjust constraint.
>         (stack_protect_testsi): Support prefixed instruction emission and
>         incorporate to stack_protect_test<mode>.
>         (stack_protect_testdi): Rename to ...
>         (stack_protect_test<mode>): ... this, adjust constraint.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.target/powerpc/pr111367.C: New test.
> ---
>  gcc/config/rs6000/rs6000.md                 | 73 ++++++++-------------
>  gcc/testsuite/g++.target/powerpc/pr111367.C | 22 +++++++
>  2 files changed, 49 insertions(+), 46 deletions(-)
>  create mode 100644 gcc/testsuite/g++.target/powerpc/pr111367.C
>
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 1a9a7b1a479..0ac79fc7735 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -12389,33 +12389,26 @@ (define_expand "stack_protect_set"
>    DONE;
>  })
>
> -(define_insn "stack_protect_setsi"
> -  [(set (match_operand:SI 0 "memory_operand" "=m")
> -       (unspec:SI [(match_operand:SI 1 "memory_operand" "m")]
> UNSPEC_SP_SET))
> -   (set (match_scratch:SI 2 "=&r") (const_int 0))]
> -  "TARGET_32BIT"
> -  "lwz%U1%X1 %2,%1\;stw%U0%X0 %2,%0\;li %2,0"
> -  [(set_attr "type" "three")
> -   (set_attr "length" "12")])
> -
>  ;; We can't use the prefixed attribute here because there are two memory
>  ;; instructions.  We can't split the insn due to the fact that this
> operation
>  ;; needs to be done in one piece.
> -(define_insn "stack_protect_setdi"
> -  [(set (match_operand:DI 0 "memory_operand" "=Y")
> -       (unspec:DI [(match_operand:DI 1 "memory_operand" "Y")]
> UNSPEC_SP_SET))
> -   (set (match_scratch:DI 2 "=&r") (const_int 0))]
> -  "TARGET_64BIT"
> +(define_insn "stack_protect_set<mode>"
> +  [(set (match_operand:P 0 "memory_operand" "=YZ")
> +       (unspec:P [(match_operand:P 1 "memory_operand" "YZ")]
> UNSPEC_SP_SET))
> +   (set (match_scratch:P 2 "=&r") (const_int 0))]
> +  ""
>  {
> -  if (prefixed_memory (operands[1], DImode))
> -    output_asm_insn ("pld %2,%1", operands);
> +  if (prefixed_memory (operands[1], <MODE>mode))
> +    /* Prefixed load only supports D-form but no update and X-form.  */
> +    output_asm_insn ("p<ptrload> %2,%1", operands);
>    else
> -    output_asm_insn ("ld%U1%X1 %2,%1", operands);
> +    output_asm_insn ("<ptrload>%U1%X1 %2,%1", operands);
>
> -  if (prefixed_memory (operands[0], DImode))
> -    output_asm_insn ("pstd %2,%0", operands);
> +  if (prefixed_memory (operands[0], <MODE>mode))
> +    /* Prefixed store only supports D-form but no update and X-form.  */
> +    output_asm_insn ("pst<wd> %2,%0", operands);
>    else
> -    output_asm_insn ("std%U0%X0 %2,%0", operands);
> +    output_asm_insn ("st<wd>%U0%X0 %2,%0", operands);
>
>    return "li %2,0";
>  }
> @@ -12461,45 +12454,33 @@ (define_expand "stack_protect_test"
>    DONE;
>  })
>
> -(define_insn "stack_protect_testsi"
> -  [(set (match_operand:CCEQ 0 "cc_reg_operand" "=x,?y")
> -        (unspec:CCEQ [(match_operand:SI 1 "memory_operand" "m,m")
> -                     (match_operand:SI 2 "memory_operand" "m,m")]
> -                    UNSPEC_SP_TEST))
> -   (set (match_scratch:SI 4 "=r,r") (const_int 0))
> -   (clobber (match_scratch:SI 3 "=&r,&r"))]
> -  "TARGET_32BIT"
> -  "@
> -   lwz%U1%X1 %3,%1\;lwz%U2%X2 %4,%2\;xor. %3,%3,%4\;li %4,0
> -   lwz%U1%X1 %3,%1\;lwz%U2%X2 %4,%2\;cmplw %0,%3,%4\;li %3,0\;li %4,0"
> -  [(set_attr "length" "16,20")])
> -
>  ;; We can't use the prefixed attribute here because there are two memory
>  ;; instructions.  We can't split the insn due to the fact that this
> operation
>  ;; needs to be done in one piece.
> -(define_insn "stack_protect_testdi"
> +(define_insn "stack_protect_test<mode>"
>    [(set (match_operand:CCEQ 0 "cc_reg_operand" "=x,?y")
> -        (unspec:CCEQ [(match_operand:DI 1 "memory_operand" "Y,Y")
> -                     (match_operand:DI 2 "memory_operand" "Y,Y")]
> +        (unspec:CCEQ [(match_operand:P 1 "memory_operand" "YZ,YZ")
> +                     (match_operand:P 2 "memory_operand" "YZ,YZ")]
>                      UNSPEC_SP_TEST))
> -   (set (match_scratch:DI 4 "=r,r") (const_int 0))
> -   (clobber (match_scratch:DI 3 "=&r,&r"))]
> -  "TARGET_64BIT"
> +   (set (match_scratch:P 4 "=r,r") (const_int 0))
> +   (clobber (match_scratch:P 3 "=&r,&r"))]
> +  ""
>  {
> -  if (prefixed_memory (operands[1], DImode))
> -    output_asm_insn ("pld %3,%1", operands);
> +  if (prefixed_memory (operands[1], <MODE>mode))
> +    /* Prefixed load only supports D-form but no update and X-form.  */
> +    output_asm_insn ("p<ptrload> %3,%1", operands);
>    else
> -    output_asm_insn ("ld%U1%X1 %3,%1", operands);
> +    output_asm_insn ("<ptrload>%U1%X1 %3,%1", operands);
>
> -  if (prefixed_memory (operands[2], DImode))
> -    output_asm_insn ("pld %4,%2", operands);
> +  if (prefixed_memory (operands[2], <MODE>mode))
> +    output_asm_insn ("p<ptrload> %4,%2", operands);
>    else
> -    output_asm_insn ("ld%U2%X2 %4,%2", operands);
> +    output_asm_insn ("<ptrload>%U2%X2 %4,%2", operands);
>
>    if (which_alternative == 0)
>      output_asm_insn ("xor. %3,%3,%4", operands);
>    else
> -    output_asm_insn ("cmpld %0,%3,%4\;li %3,0", operands);
> +    output_asm_insn ("cmpl<wd> %0,%3,%4\;li %3,0", operands);
>
>    return "li %4,0";
>  }
> diff --git a/gcc/testsuite/g++.target/powerpc/pr111367.C
> b/gcc/testsuite/g++.target/powerpc/pr111367.C
> new file mode 100644
> index 00000000000..8f9d4415672
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/powerpc/pr111367.C
> @@ -0,0 +1,22 @@
> +/* { dg-do assemble } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -fstack-protector-strong" } */
> +
> +/* Verify object file can be generated successfully.  */
> +
> +struct SortAscending
> +{
> +};
> +
> +typedef unsigned long long size_t;
> +
> +void VQSort (long long *, size_t, SortAscending);
> +
> +void
> +BenchAllColdSort ()
> +{
> +  typedef long long T;
> +  constexpr size_t kSize = 10 * 1000;
> +  alignas (16) T items[kSize];
> +  VQSort (items, kSize, SortAscending ());
> +}
> --
> 2.40.1
>