Fix power10 fusion bug with prefixed loads, PR target/105325

This changes fixes PR target/105325.  PR target/105325 is a bug where an
invalid lwa instruction is generated due to power10 fusion of a load
instruction to a GPR and an compare immediate instruction with the immediate
being -1, 0, or 1.

In some cases, when the load instruction is done, the GCC compiler would
generate a load instruction with an offset that was too large to fit into the
normal load instruction.

In particular, loads from the stack might originally have a small offset, so
that the load is not a prefixed load.  However, after the stack is set up, and
register allocation has been done, the offset now is large enough that we would
have to use a prefixed load instruction.

The support for prefixed loads did not consider that patterns with a fused load
and compare might have a prefixed address.  Without this support, the proper
prefixed load won't be generated.

In the original code, when the split2 pass is run after reload has finished the
ds_form_mem_operand predicate that was used for lwa and ld no longer returns
true.  When the pattern was created, ds_form_mem_operand recognized the insn as
being valid since the offset was small.  But after register allocation,
ds_form_mem_operand did not return true.  Because it didn't return true, the
insn could not be split.  Since the insn was not split and the prefix support
did not indicate a prefixed instruction was used, the wrong load is generated.

The solution involves:

    1)	Don't use ds_form_mem_operand for ld and lwa, always use
	non_update_memory_operand.

    2)	Delete ds_form_mem_operand since it is no longer used.

    3)	Use the "YZ" constraints for ld/lwa instead of "m".

    4)	If we don't need to sign extend the lwa, convert it to lwz, and use
	cmpwi instead of cmpdi.  Adjust the insn name to reflect the code
	generate.

    5)	Insure that the insn using lwa will be recognized as having a prefixed
	operand (and hence the insn length will be 16 bytes instead of 8
	bytes).

	5a) Set the prefixed and maybe_prefix attributes to know that
	    fused_load_cmpi are also load insns;

	5b) In the case where we are just setting CC and not using the memory
	    afterward, set the clobber to use a DI register, and put an
	    explicit sign_extend operation in the split;

	5c) Set the sign_extend attribute to "yes" for lwa.

	5d) 5a-5c are the things that prefixed_load_p in rs6000.cc checks to
	    ensure that lwa is treated as a ds-form instruction and not as
	    a d-form instruction (i.e. lwz).

    6)	Add a new test case for this case.

    7)	Adjust the insn counts in fusion-p10-ldcmpi.c.  Because we are no
	longer using ds_form_mem_operand, the ld and lwa instructions will fuse
	x-form (reg+reg) addresses in addition ds-form (reg+offset or reg).

2023-06-23   Michael Meissner  <meissner@linux.ibm.com>

gcc/

	PR target/105325
	* 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/predicates.md (ds_form_mem_operand): Delete.
	* config/rs6000/rs6000.md (prefixed attribute): Add support for load
	plus compare immediate fused insns.
	(maybe_prefixed): Likewise.

gcc/testsuite/

	PR target/105325
	* g++.target/powerpc/pr105325.C: New test.
	* gcc.target/powerpc/fusion-p10-ldcmpi.c: Update insn counts.

Co-Authored-By: Aaron Sawdey  <acsawdey@linux.ibm.com>

diff --git a/gcc/config/rs6000/fusion.md b/gcc/config/rs6000/fusion.md
index d45fb138a702d890425cf2b3a3c67b5717a57a80..e286bf526a2f3cda5728208c40e156966b5946bf 100644 (file)
--- 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 "non_update_memory_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 "non_update_memory_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 "non_update_memory_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 "non_update_memory_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)"
@@ -104,17 +104,17 @@
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
 ;; 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"
+(define_insn_and_split "*lwz_cmpwi_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 "non_update_memory_operand" "m")
                     (match_operand:SI 3 "const_m1_to_1_operand" "n")))
    (clobber (match_scratch:SI 0 "=r"))]
   "(TARGET_P10_FUSION)"
-  "lwa%X1 %0,%1\;cmpdi %2,%0,%3"
+  "lwz%X1 %0,%1\;cmpwi %2,%0,%3"
   "&& 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))"
+                                      SImode, NON_PREFIXED_D))"
   [(set (match_dup 0) (match_dup 1))
    (set (match_dup 2)
         (compare:CC (match_dup 0) (match_dup 3)))]
@@ -146,17 +146,17 @@
 
 ;; load-cmpi fusion pattern generated by gen_ld_cmpi_p10
 ;; 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"
+(define_insn_and_split "*lwz_cmpwi_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 "non_update_memory_operand" "m")
                     (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)"
-  "lwa%X1 %0,%1\;cmpdi %2,%0,%3"
+  "lwz%X1 %0,%1\;cmpwi %2,%0,%3"
   "&& 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))"
+                                      SImode, NON_PREFIXED_D))"
   [(set (match_dup 0) (match_dup 1))
    (set (match_dup 2)
         (compare:CC (match_dup 0) (match_dup 3)))]
@@ -190,7 +190,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 "non_update_memory_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)"
@@ -205,6 +205,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 82e8f863b02c8b7b8fa1d8e6b9c597f8f9e0ebd9..4d1f825ad904dee28b74ce990b9af3e3388d6e62 100755 (executable)
--- a/gcc/config/rs6000/genfusion.pl
+++ b/gcc/config/rs6000/genfusion.pl
@@ -61,20 +61,31 @@ sub gen_ld_cmpi_p10_one
   my $mempred = "non_update_memory_operand";
   my $extend;
 
+  # We need to special case lwa.  The prefixed_load_p function in rs6000.cc
+  # (which determines if a load instruction is prefixed) uses the fact that the
+  # register mode is different from the memory mode, and that the sign_extend
+  # attribute is set to use DS-form rules for the address instead of D-form.
+  # If the register size is the same, prefixed_load_p assumes we are doing a
+  # lwz.  We change to use an lwz and word compare if we don't need to sign
+  # extend the SImode value.  Otherwise if we need the value, we need to
+  # make sure the insn is marked as ds-form.
+  my $cmp_size_char = ($lmode eq "SI"
+                      && $ccmode eq "CC"
+                      && $result !~ /^EXT|^DI$/) ? "w" : "d";
+
   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";
+    ($lmode eq "DI") and $np = "NON_PREFIXED_DS";
+    ($lmode eq "SI" && $cmp_size_char eq "d") and $np = "NON_PREFIXED_DS";
   } else {
     if ($lmode eq "DI") {
       # ld is DS-form, but lwz is not.
       $np = "NON_PREFIXED_DS";
-      $mempred = "ds_form_mem_operand";
     }
   }
 
   my $cmpl = ($ccmode eq "CC") ? "" : "l";
-  my $echr = ($ccmode eq "CC") ? "a" : "z";
+  my $echr = ($ccmode eq "CC" && $cmp_size_char eq "d") ? "a" : "z";
   if ($lmode eq "DI") { $echr = ""; }
   my $constpred = ($ccmode eq "CC") ? "const_m1_to_1_operand"
                                    : "const_0_to_1_operand";
@@ -91,12 +102,15 @@ sub gen_ld_cmpi_p10_one
   }
 
   my $ldst = mode_to_ldst_char($lmode);
+
+  # DS-form addresses need YZ, and not m.
+  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}"
+(define_insn_and_split "*l${ldst}${echr}_cmp${cmpl}${cmp_size_char}i_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;
@@ -119,7 +133,7 @@ HERE
 
   print <<HERE;
   "(TARGET_P10_FUSION)"
-  "l${ldst}${echr}%X1 %0,%1\\;cmp${cmpl}di %2,%0,%3"
+  "l${ldst}${echr}%X1 %0,%1\\;cmp${cmpl}${cmp_size_char}i %2,%0,%3"
   "&& reload_completed
    && (cc_reg_not_cr0_operand (operands[2], CCmode)
        || !address_is_non_pfx_d_or_x (XEXP (operands[1], 0),
@@ -140,6 +154,15 @@ HERE
   ""
   [(set_attr "type" "fused_load_cmpi")
    (set_attr "cost" "8")
+HERE
+
+  if ($lmode eq "SI" && $ccmode eq "CC" && $cmp_size_char eq "d") {
+    # prefixed_load_p needs the sign_extend attribute to validate lwa as a
+    # DS-form instruction instead of D-form.
+    print "   (set_attr \"sign_extend\" \"yes\")\n";
+  }
+
+  print <<HERE
    (set_attr "length" "8")])
 
 HERE

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index a16ee30f0c061965da07a5832097eeffa6ccf29c..6b564837c6eac154dc9eca7e03c6fb5e4c56a675 100644 (file)
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1125,20 +1125,6 @@
   return INTVAL (offset) % 4 == 0;
 })
 
-;; Return 1 if the operand is a memory operand that has a valid address for
-;; a DS-form instruction. I.e. the address has to be either just a register,
-;; or register + const where the two low order bits of const are zero.
-(define_predicate "ds_form_mem_operand"
-  (match_code "subreg,mem")
-{
-  if (!any_memory_operand (op, mode))
-    return false;
-
-  rtx addr = XEXP (op, 0);
-
-  return address_to_insn_form (addr, mode, NON_PREFIXED_DS) == INSN_FORM_DS;
-})
-
 ;; Return 1 if the operand, used inside a MEM, is a SYMBOL_REF.
 (define_predicate "symbol_ref_operand"
   (and (match_code "symbol_ref")

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index b0db8ae508d8ee50e34f85b654988ae0919e152f..75c5e5fc93dd992ce64abb15e3bcf8c07fcaea49 100644 (file)
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -287,7 +287,7 @@
 ;; Whether this insn has a prefixed form and a non-prefixed form.
 (define_attr "maybe_prefixed" "no,yes"
   (if_then_else (eq_attr "type" "load,fpload,vecload,store,fpstore,vecstore,
-                                integer,add")
+                                integer,add,fused_load_cmpi")
                (const_string "yes")
                (const_string "no")))
 
@@ -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 (file)
index 0000000..18a2e52
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/pr105325.C
@@ -0,0 +1,28 @@
+/* { 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" } */
+
+/* PR target/105324.  Test that power10 fusion does not generate an LWA/CMPDI
+   with a large offset that the assembler rejects.  Instead it should a
+   PLWZ/CMPWI combination.
+
+   Originally, 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 (;;)
+      ;
+}

diff --git a/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c b/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
index 526a026d8741b190a4cd012b877f4dc5b7aa3936..165bd9a07add4e136f4ef31f1e200a16171bc7ab 100644 (file)
--- a/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
+++ b/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
@@ -54,15 +54,17 @@ TEST(uint8_t)
 TEST(int8_t)
 
 /* { dg-final { scan-assembler-times "lbz_cmpldi_cr0_QI_clobber_CCUNS_zero"   4 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_DI_CC_none"             4 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_clobber_CC_none"        4 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_DI_CCUNS_none"         1 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_clobber_CCUNS_none"    1 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_DI_CC_none"            24 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_clobber_CC_none"        8 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_DI_CCUNS_none"         2 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_clobber_CCUNS_none"    2 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lha_cmpdi_cr0_HI_clobber_CC_sign"      16 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lhz_cmpldi_cr0_HI_clobber_CCUNS_zero"   4 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_EXTSI_CC_sign"         0 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_clobber_CC_none"       4 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpwi_cr0_SI_clobber_CC_none"       8 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpwi_cr0_SI_SI_CC_none"            8 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_EXTSI_CCUNS_zero"     0 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_SI_CCUNS_none"        2 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_clobber_CCUNS_none"   2 { target lp64 } } } */
 
 /* { dg-final { scan-assembler-times "lbz_cmpldi_cr0_QI_clobber_CCUNS_zero"   2 { target ilp32 } } } */
@@ -73,6 +75,8 @@ TEST(int8_t)
 /* { dg-final { scan-assembler-times "lha_cmpdi_cr0_HI_clobber_CC_sign"       8 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "lhz_cmpldi_cr0_HI_clobber_CCUNS_zero"   2 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_EXTSI_CC_sign"         0 { target ilp32 } } } */
-/* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_clobber_CC_none"       9 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpwi_cr0_SI_SI_CC_none"           36 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpwi_cr0_SI_clobber_CC_none"      16 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_EXTSI_CCUNS_zero"     0 { target ilp32 } } } */
 /* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_clobber_CCUNS_none"   6 { target ilp32 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_SI_CCUNS_none"        2 { target ilp32 } } } */