public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Michael Meissner <meissner@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc(refs/users/meissner/heads/work122)] Fix power10 fusion and -fstack-protector, PR target/105325
Date: Fri,  9 Jun 2023 01:32:20 +0000 (GMT)	[thread overview]
Message-ID: <20230609013220.9CF783858C20@sourceware.org> (raw)

https://gcc.gnu.org/g:58463f8c025a5473e187cb1d9149cd27f71fb4eb

commit 58463f8c025a5473e187cb1d9149cd27f71fb4eb
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Thu Jun 8 21:32:01 2023 -0400

    Fix power10 fusion and -fstack-protector, PR target/105325
    
    This patch fixes an issue where if you use the -fstack-protector and
    -mcpu=power10 options and you have a large stack frame, the GCC compiler will
    generate a LWA instruction with a large offset.
    
    There are several problems with the current GCC:
    
        1)  The prefixed attribute was not checking insns with the type
            fused_load_cmpi for being load insns.  This meant if the load + compare
            fusion was not split, the insn size might be off, and the wrong
            instruction might be generated.
    
        2)  The code in prefixed_load_p looks at the "sign_extend" attribute and
            whether the register mode was different than the memory for SImode loads
            to detect LWA instructions.
    
        3)  The constraints in fusion.md (generated by genfusion.pl) use "m" for LWA
            and LD, when they should use "YZ".
    
        4)  The code to determine whether to split the insn if it has a prefixed
            address was passing in SImode.  For LWA, we need to pass in DImode and
            not SImode to properly determine if the address is prefixed.
    
    The fix is to modify genfusion.pl that it sets the "YZ" constraint instead of
    "m" for the ld and lwa instructions.
    
    I also set the signed attribute on the HI/SImode loads that do sign extension.
    
    I modified the predicate for lwa to use lwa_operand instead of the predicate
    ds_form_mem_operand.  This predicate has direct checks to prevent a prefixed lwa
    from being generated.  I also modified the condition for the insn splitter so
    that it passes DImode for the lwa instructions and not SImode.
    
    I modified the "prefixed" attribute so that it also checks fused_load_cmpi.
    
    2023-06-07   Michael Meissner  <meissner@linux.ibm.com>
    
    gcc/
    
            * config/rs6000/genfusion.pl (gen_ld_cmpi_p10_one): Fix problems that
            allowed prefixed lwa to be generated.
            * config/rs6000/fusion.md: Regenerate.
            * config/rs6000/rs6000.md (prefixed attribute): Treat fused_load_cmpi
            insns as being load insns.
    
    gcc/testsuite/
    
            * g++.target/powerpc/pr105325.C: New test.

Diff:
---
 gcc/config/rs6000/fusion.md                 | 25 +++++++++++++++----------
 gcc/config/rs6000/genfusion.pl              | 27 ++++++++++++++++++++++-----
 gcc/config/rs6000/rs6000.md                 |  2 +-
 gcc/testsuite/g++.target/powerpc/pr105325.C | 26 ++++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/gcc/config/rs6000/fusion.md b/gcc/config/rs6000/fusion.md
index d45fb138a70..b7c9035e882 100644
--- a/gcc/config/rs6000/fusion.md
+++ b/gcc/config/rs6000/fusion.md
@@ -22,7 +22,7 @@
 ;; load mode is DI result mode is clobber compare mode is CC extend is none
 (define_insn_and_split "*ld_cmpdi_cr0_DI_clobber_CC_none"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x")
-        (compare:CC (match_operand:DI 1 "ds_form_mem_operand" "m")
+        (compare:CC (match_operand:DI 1 "ds_form_mem_operand" "YZ")
                     (match_operand:DI 3 "const_m1_to_1_operand" "n")))
    (clobber (match_scratch:DI 0 "=r"))]
   "(TARGET_P10_FUSION)"
@@ -43,7 +43,7 @@
 ;; load mode is DI result mode is clobber compare mode is CCUNS extend is none
 (define_insn_and_split "*ld_cmpldi_cr0_DI_clobber_CCUNS_none"
   [(set (match_operand:CCUNS 2 "cc_reg_operand" "=x")
-        (compare:CCUNS (match_operand:DI 1 "ds_form_mem_operand" "m")
+        (compare:CCUNS (match_operand:DI 1 "ds_form_mem_operand" "YZ")
                        (match_operand:DI 3 "const_0_to_1_operand" "n")))
    (clobber (match_scratch:DI 0 "=r"))]
   "(TARGET_P10_FUSION)"
@@ -64,7 +64,7 @@
 ;; load mode is DI result mode is DI compare mode is CC extend is none
 (define_insn_and_split "*ld_cmpdi_cr0_DI_DI_CC_none"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x")
-        (compare:CC (match_operand:DI 1 "ds_form_mem_operand" "m")
+        (compare:CC (match_operand:DI 1 "ds_form_mem_operand" "YZ")
                     (match_operand:DI 3 "const_m1_to_1_operand" "n")))
    (set (match_operand:DI 0 "gpc_reg_operand" "=r") (match_dup 1))]
   "(TARGET_P10_FUSION)"
@@ -85,7 +85,7 @@
 ;; load mode is DI result mode is DI compare mode is CCUNS extend is none
 (define_insn_and_split "*ld_cmpldi_cr0_DI_DI_CCUNS_none"
   [(set (match_operand:CCUNS 2 "cc_reg_operand" "=x")
-        (compare:CCUNS (match_operand:DI 1 "ds_form_mem_operand" "m")
+        (compare:CCUNS (match_operand:DI 1 "ds_form_mem_operand" "YZ")
                        (match_operand:DI 3 "const_0_to_1_operand" "n")))
    (set (match_operand:DI 0 "gpc_reg_operand" "=r") (match_dup 1))]
   "(TARGET_P10_FUSION)"
@@ -106,7 +106,7 @@
 ;; load mode is SI result mode is clobber compare mode is CC extend is none
 (define_insn_and_split "*lwa_cmpdi_cr0_SI_clobber_CC_none"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x")
-        (compare:CC (match_operand:SI 1 "ds_form_mem_operand" "m")
+        (compare:CC (match_operand:SI 1 "lwa_operand" "YZ")
                     (match_operand:SI 3 "const_m1_to_1_operand" "n")))
    (clobber (match_scratch:SI 0 "=r"))]
   "(TARGET_P10_FUSION)"
@@ -114,13 +114,14 @@
   "&& reload_completed
    && (cc_reg_not_cr0_operand (operands[2], CCmode)
        || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),
-                                      SImode, NON_PREFIXED_DS))"
+                                      DImode, NON_PREFIXED_DS))"
   [(set (match_dup 0) (match_dup 1))
    (set (match_dup 2)
         (compare:CC (match_dup 0) (match_dup 3)))]
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+   (set_attr "sign_extend" "yes")
    (set_attr "length" "8")])
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
@@ -148,7 +149,7 @@
 ;; load mode is SI result mode is SI compare mode is CC extend is none
 (define_insn_and_split "*lwa_cmpdi_cr0_SI_SI_CC_none"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x")
-        (compare:CC (match_operand:SI 1 "ds_form_mem_operand" "m")
+        (compare:CC (match_operand:SI 1 "lwa_operand" "YZ")
                     (match_operand:SI 3 "const_m1_to_1_operand" "n")))
    (set (match_operand:SI 0 "gpc_reg_operand" "=r") (match_dup 1))]
   "(TARGET_P10_FUSION)"
@@ -156,13 +157,14 @@
   "&& reload_completed
    && (cc_reg_not_cr0_operand (operands[2], CCmode)
        || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),
-                                      SImode, NON_PREFIXED_DS))"
+                                      DImode, NON_PREFIXED_DS))"
   [(set (match_dup 0) (match_dup 1))
    (set (match_dup 2)
         (compare:CC (match_dup 0) (match_dup 3)))]
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+   (set_attr "sign_extend" "yes")
    (set_attr "length" "8")])
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
@@ -190,7 +192,7 @@
 ;; load mode is SI result mode is EXTSI compare mode is CC extend is sign
 (define_insn_and_split "*lwa_cmpdi_cr0_SI_EXTSI_CC_sign"
   [(set (match_operand:CC 2 "cc_reg_operand" "=x")
-        (compare:CC (match_operand:SI 1 "ds_form_mem_operand" "m")
+        (compare:CC (match_operand:SI 1 "lwa_operand" "YZ")
                     (match_operand:SI 3 "const_m1_to_1_operand" "n")))
    (set (match_operand:EXTSI 0 "gpc_reg_operand" "=r") (sign_extend:EXTSI (match_dup 1)))]
   "(TARGET_P10_FUSION)"
@@ -198,13 +200,14 @@
   "&& reload_completed
    && (cc_reg_not_cr0_operand (operands[2], CCmode)
        || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),
-                                      SImode, NON_PREFIXED_DS))"
+                                      DImode, NON_PREFIXED_DS))"
   [(set (match_dup 0) (sign_extend:EXTSI (match_dup 1)))
    (set (match_dup 2)
         (compare:CC (match_dup 0) (match_dup 3)))]
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+   (set_attr "sign_extend" "yes")
    (set_attr "length" "8")])
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
@@ -247,6 +250,7 @@
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+   (set_attr "sign_extend" "yes")
    (set_attr "length" "8")])
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
@@ -289,6 +293,7 @@
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+   (set_attr "sign_extend" "yes")
    (set_attr "length" "8")])
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
diff --git a/gcc/config/rs6000/genfusion.pl b/gcc/config/rs6000/genfusion.pl
index 82e8f863b02..599bd3ad19b 100755
--- a/gcc/config/rs6000/genfusion.pl
+++ b/gcc/config/rs6000/genfusion.pl
@@ -57,14 +57,23 @@ sub gen_ld_cmpi_p10_one
 {
   my ($lmode, $result, $ccmode) = @_;
 
+  my $lmode_prefix_call = $lmode;
   my $np = "NON_PREFIXED_D";
   my $mempred = "non_update_memory_operand";
   my $extend;
 
   if ($ccmode eq "CC") {
-    # ld and lwa are both DS-FORM.
-    ($lmode =~ /^[SD]I$/) and $np = "NON_PREFIXED_DS";
-    ($lmode =~ /^[SD]I$/) and $mempred = "ds_form_mem_operand";
+    # ld and lwa are both DS-FORM.  lwa needs to use lwa_operand and also
+    # call address_is_non_pfx_d_or_x with DImode, not SImode to properly
+    # handle recognizing whether the address is prefixed.
+    if ($lmode eq "DI") {
+      $np = "NON_PREFIXED_DS";
+      $mempred = "ds_form_mem_operand";
+    } elsif ($lmode eq "SI") {
+      $np = "NON_PREFIXED_DS";
+      $mempred = "lwa_operand";
+      $lmode_prefix_call = "DI";
+    }
   } else {
     if ($lmode eq "DI") {
       # ld is DS-form, but lwz is not.
@@ -91,12 +100,13 @@ sub gen_ld_cmpi_p10_one
   }
 
   my $ldst = mode_to_ldst_char($lmode);
+  my $constraint = ($np eq "NON_PREFIXED_DS") ? "YZ" : "m";
   print <<HERE;
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
 ;; load mode is $lmode result mode is $result compare mode is $ccmode extend is $extend
 (define_insn_and_split "*l${ldst}${echr}_cmp${cmpl}di_cr0_${lmode}_${result}_${ccmode}_${extend}"
   [(set (match_operand:${ccmode} 2 "cc_reg_operand" "=x")
-        (compare:${ccmode} (match_operand:${lmode} 1 "${mempred}" "m")
+        (compare:${ccmode} (match_operand:${lmode} 1 "${mempred}" "${constraint}")
 HERE
   print "   " if $ccmode eq "CCUNS";
 print <<HERE;
@@ -123,7 +133,7 @@ HERE
   "&& reload_completed
    && (cc_reg_not_cr0_operand (operands[2], CCmode)
        || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),
-                                      ${lmode}mode, ${np}))"
+                                      ${lmode_prefix_call}mode, ${np}))"
 HERE
 
   if ($extend eq "none") {
@@ -140,6 +150,13 @@ HERE
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+HERE
+
+  if ($ccmode eq "CC" && $lmode ne "DI") {
+    print "   (set_attr \"sign_extend\" \"yes\")\n";
+  }
+
+  print <<HERE;
    (set_attr "length" "8")])
 
 HERE
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index b0db8ae508d..ed2f06e56ac 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -302,7 +302,7 @@
 	      (eq_attr "maybe_prefixed" "no"))
 	 (const_string "no")
 
-	 (eq_attr "type" "load,fpload,vecload")
+	 (eq_attr "type" "load,fpload,vecload,fused_load_cmpi")
 	 (if_then_else (match_test "prefixed_load_p (insn)")
 		       (const_string "yes")
 		       (const_string "no"))
diff --git a/gcc/testsuite/g++.target/powerpc/pr105325.C b/gcc/testsuite/g++.target/powerpc/pr105325.C
new file mode 100644
index 00000000000..d0e66a0b897
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/pr105325.C
@@ -0,0 +1,26 @@
+/* { dg-do assemble } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-require-effective-target powerpc_prefixed_addr } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10 -fstack-protector" } */
+
+/* Test that power10 fusion does not generate an LWA/CMPDI instruction pair
+   instead of PLWZ/CMPWI.  Ultimately the code was dying because the fusion
+   load + compare -1/0/1 patterns did not handle the possibility that the load
+   might be prefixed.  The -fstack-protector option is needed to show the
+   bug.  */
+
+struct Ath__array1D {
+  int _current;
+  int getCnt() { return _current; }
+};
+struct extMeasure {
+  int _mapTable[10000];
+  Ath__array1D _metRCTable;
+};
+void measureRC() {
+  extMeasure m;
+  for (; m._metRCTable.getCnt();)
+    for (;;)
+      ;
+}

             reply	other threads:[~2023-06-09  1:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-09  1:32 Michael Meissner [this message]
  -- strict thread matches above, loose matches on Subject: below --
2023-06-13 23:24 Michael Meissner
2023-06-13 17:21 Michael Meissner
2023-06-13  2:57 Michael Meissner
2023-06-09 21:04 Michael Meissner
2023-06-09  6:08 Michael Meissner
2023-06-09  4:17 Michael Meissner
2023-06-08 21:20 Michael Meissner
2023-06-08 20:23 Michael Meissner
2023-06-08 16:53 Michael Meissner
2023-06-08 14:38 Michael Meissner
2023-06-07 20:58 Michael Meissner
2023-06-07 18:56 Michael Meissner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230609013220.9CF783858C20@sourceware.org \
    --to=meissner@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).