* [PATCH] rs6000: Make 32 bit stack_protect support prefixed insn [PR111367]
@ 2023-09-27 5:38 Kewen.Lin
2023-10-03 13:59 ` David Edelsohn
0 siblings, 1 reply; 2+ messages in thread
From: Kewen.Lin @ 2023-09-27 5:38 UTC (permalink / raw)
To: GCC Patches
Cc: Segher Boessenkool, David Edelsohn, Peter Bergner, Michael Meissner
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
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] rs6000: Make 32 bit stack_protect support prefixed insn [PR111367]
2023-09-27 5:38 [PATCH] rs6000: Make 32 bit stack_protect support prefixed insn [PR111367] Kewen.Lin
@ 2023-10-03 13:59 ` David Edelsohn
0 siblings, 0 replies; 2+ messages in thread
From: David Edelsohn @ 2023-10-03 13:59 UTC (permalink / raw)
To: Kewen.Lin
Cc: GCC Patches, Segher Boessenkool, Peter Bergner, Michael Meissner
[-- Attachment #1: Type: text/plain, Size: 7505 bytes --]
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-10-03 14:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 5:38 [PATCH] rs6000: Make 32 bit stack_protect support prefixed insn [PR111367] Kewen.Lin
2023-10-03 13:59 ` David Edelsohn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).