From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) by sourceware.org (Postfix) with ESMTPS id F20D23858C62 for ; Tue, 3 Oct 2023 14:00:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F20D23858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x634.google.com with SMTP id a640c23a62f3a-9ada2e6e75fso175355466b.2 for ; Tue, 03 Oct 2023 07:00:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696341604; x=1696946404; darn=gcc.gnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=xI+uiMjKcYPcIvoo0LTH6PJJxsxEUxI4uZNf7fAxOpI=; b=GfIsDlGzm9pGB7N/+nvH+JUb80ykoVePaOwmt4OSVsaHiuJdVBTOMJETxXGMa+/n9P AH1G/6T+q6tpiGSfpDF4jKZeiKKp9mIfH4YgoFnE6FzlP126Yp+7bT2qrpHfU7Wshle1 MjoJzgUB53plDYYjWyxMj6UvQsojDWNxrepdaGA27wnwgK2JYoVDhhLLZw3I+FxJQgCh vZB5/B2Fjr15unBOOubbQCpZJ/ZCjHudjuc5Y69lxmWcYCXM3M4qLCRy4dbhJA9DrJRI XsIG5IWgFU3XS9HumCPG+drgYjcn3QQeLKu3ljRHUdUV/IuLOlnGv4R7CGcqOlkGDRGt jn1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696341604; x=1696946404; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=xI+uiMjKcYPcIvoo0LTH6PJJxsxEUxI4uZNf7fAxOpI=; b=FkdRtE8iKjt+mYb63yniAfWynDNjcy9leSIw47EwyQzkMfGLBQVOY75y2KihxQhUwB vni50Ri0YCSE4jqYjG1n9I+fMi8CF2f5/vf1+fsjbBE1WWAY/Leg3kM0YxuBGMpQhKcc 3hU2pzaQ+JRStx1CofJE7HaiqqdV/5IWAATJ/bKDn7FUZrm0aLPpy0KXM5Qd25VydfOH 0Jqz7sVUWwIw7QiV2+X3pXEwpnbUONfJGdgDS2S1P3k5vsbtsjibeO/TRinvkPVo8FW9 et7ot9iKqILz5Z3r777KsEmCflYpBnt3RmzO3jFVy84YbxnA6ZELo/i/vAggLSUZKAxT fCkA== X-Gm-Message-State: AOJu0YwwW2vboJxFDcZ6ggjWzwkdfv7Ar21jsDihxgb4AEtWfn/qd60q ewYS69UQgmjO2tl/6O/f+X6zMpNBjqfvlm1CQek= X-Google-Smtp-Source: AGHT+IHF6hGqu7iQ5VeyNCKdKlYcml2ttS+iUw2hBcnqV8vecJ3uYE6Re7UPoXv4sYTIooMwooLQLPYT+1LjYWRQL/8= X-Received: by 2002:a17:906:292a:b0:9a5:c9a8:1816 with SMTP id v10-20020a170906292a00b009a5c9a81816mr4045382ejd.58.1696341604266; Tue, 03 Oct 2023 07:00:04 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: David Edelsohn Date: Tue, 3 Oct 2023 09:59:52 -0400 Message-ID: Subject: Re: [PATCH] rs6000: Make 32 bit stack_protect support prefixed insn [PR111367] To: "Kewen.Lin" Cc: GCC Patches , Segher Boessenkool , Peter Bergner , Michael Meissner Content-Type: multipart/alternative; boundary="000000000000bd76ad0606d04e18" X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --000000000000bd76ad0606d04e18 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Sep 27, 2023 at 1:38=E2=80=AFAM Kewen.Lin wro= te: > 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. > (stack_protect_setdi): Rename to ... > (stack_protect_set): ... this, adjust constraint. > (stack_protect_testsi): Support prefixed instruction emission and > incorporate to stack_protect_test. > (stack_protect_testdi): Rename to ... > (stack_protect_test): ... 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" "=3Dm") > - (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] > UNSPEC_SP_SET)) > - (set (match_scratch:SI 2 "=3D&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" "=3DY") > - (unspec:DI [(match_operand:DI 1 "memory_operand" "Y")] > UNSPEC_SP_SET)) > - (set (match_scratch:DI 2 "=3D&r") (const_int 0))] > - "TARGET_64BIT" > +(define_insn "stack_protect_set" > + [(set (match_operand:P 0 "memory_operand" "=3DYZ") > + (unspec:P [(match_operand:P 1 "memory_operand" "YZ")] > UNSPEC_SP_SET)) > + (set (match_scratch:P 2 "=3D&r") (const_int 0))] > + "" > { > - if (prefixed_memory (operands[1], DImode)) > - output_asm_insn ("pld %2,%1", operands); > + if (prefixed_memory (operands[1], mode)) > + /* Prefixed load only supports D-form but no update and X-form. */ > + output_asm_insn ("p %2,%1", operands); > else > - output_asm_insn ("ld%U1%X1 %2,%1", operands); > + output_asm_insn ("%U1%X1 %2,%1", operands); > > - if (prefixed_memory (operands[0], DImode)) > - output_asm_insn ("pstd %2,%0", operands); > + if (prefixed_memory (operands[0], mode)) > + /* Prefixed store only supports D-form but no update and X-form. */ > + output_asm_insn ("pst %2,%0", operands); > else > - output_asm_insn ("std%U0%X0 %2,%0", operands); > + output_asm_insn ("st%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" "=3Dx,?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 "=3Dr,r") (const_int 0)) > - (clobber (match_scratch:SI 3 "=3D&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" > [(set (match_operand:CCEQ 0 "cc_reg_operand" "=3Dx,?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 "=3Dr,r") (const_int 0)) > - (clobber (match_scratch:DI 3 "=3D&r,&r"))] > - "TARGET_64BIT" > + (set (match_scratch:P 4 "=3Dr,r") (const_int 0)) > + (clobber (match_scratch:P 3 "=3D&r,&r"))] > + "" > { > - if (prefixed_memory (operands[1], DImode)) > - output_asm_insn ("pld %3,%1", operands); > + if (prefixed_memory (operands[1], mode)) > + /* Prefixed load only supports D-form but no update and X-form. */ > + output_asm_insn ("p %3,%1", operands); > else > - output_asm_insn ("ld%U1%X1 %3,%1", operands); > + output_asm_insn ("%U1%X1 %3,%1", operands); > > - if (prefixed_memory (operands[2], DImode)) > - output_asm_insn ("pld %4,%2", operands); > + if (prefixed_memory (operands[2], mode)) > + output_asm_insn ("p %4,%2", operands); > else > - output_asm_insn ("ld%U2%X2 %4,%2", operands); > + output_asm_insn ("%U2%X2 %4,%2", operands); > > if (which_alternative =3D=3D 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 %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=3Dpower10 -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 =3D 10 * 1000; > + alignas (16) T items[kSize]; > + VQSort (items, kSize, SortAscending ()); > +} > -- > 2.40.1 > --000000000000bd76ad0606d04e18--