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)] Update ChangeLog.meissner
Date: Tue, 13 Jun 2023 17:23:22 +0000 (GMT)	[thread overview]
Message-ID: <20230613172322.B2E133858D38@sourceware.org> (raw)

https://gcc.gnu.org/g:6b3659d8b2093c33897ff3e4c5a9ef538e96c007

commit 6b3659d8b2093c33897ff3e4c5a9ef538e96c007
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Tue Jun 13 13:23:19 2023 -0400

    Update ChangeLog.meissner

Diff:
---
 gcc/ChangeLog.meissner | 80 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 63 insertions(+), 17 deletions(-)

diff --git a/gcc/ChangeLog.meissner b/gcc/ChangeLog.meissner
index 9fcb57178b8..50cef5a69f0 100644
--- a/gcc/ChangeLog.meissner
+++ b/gcc/ChangeLog.meissner
@@ -1,4 +1,4 @@
-==================== Branch work122, patch #5 ====================
+==================== Branch work122, patch #8 ====================
 
 Fix power10 fusion and -fstack-protector, PR target/105325
 
@@ -6,26 +6,67 @@ 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:
+The important thing in the bug is that -fstack-protector is used, but it could
+potentially happen with fused load-compare to any stack location when the stack
+frame is larger than 32K without -fstack-protector.
 
-    1)	The constraints in fusion.md (generated by genfusion.pl) use "m" for LWA
-	and LD, when they should use "YZ".
+What happens is the initial insn that is created is:
 
-    2)	The rules for automatically setting the prefixed attribute were not
-	checking that these fused load and compare immediate fusion operations
-	might have prefixed addresses.
+(insn 6 5 7 2 (parallel [
+            (set (reg:CC 119)
+                 (compare:CC (mem/c:SI (plus:DI (reg/f:DI 110 sfp)
+                                                (const_int -4))
+                             (const_int 0 [0])))
+            (clobber (scratch:DI))
+        ])
+     (nil))
 
-The fix is to modify genfusion.pl that it sets the "YZ" constraint instead of
-"m" for the ld and lwa instructions.
+After the stack size is finalized, the frame pointer removed, and the post
+reload phase is run, the insn is now:
 
-This patch also modifies the prefixed and maybe_prefixed attributes so that they
-check load + compare immediate instructions for be a load instruction.  The
-patch also modifies genfusion.pl so that the lwa_cmp* insns also sets things up
-so that the prefixed_load_p function declares the address to be prefixed.  These
-modifications include using a DImode scratch register instead of SImode, and
-setting the "sign_extend" attribute.
+(insn 6 5 7 2 (parallel [
+            (set (reg:CC 100 0 [119])
+                 (compare:CC (mem/c:SI (plus:DI (reg/f:DI 1 1)
+                                                (const_int 40044))
+                             (const_int 0 [0])))
+            (clobber (reg:DI 9 9 [120]))
+        ])
+     (nil))
 
-I also added a test case for this condition.
+When the split2 pass is run after reload has finished the ds_form_mem_operand
+predicate that used for lwa and ld no longer returns true.  This means that
+since the operand predicates aren't recognized, it won't be split.
+
+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 use.
+
+    3)	Use the "YZ" constraints for ld/lwa instead of "m".
+
+    4)	Insure that the insn will be recognized as having a prefixed operand
+	(and hence the instruction length is 16 bytes instead of 8 bytes).
+
+	4a) Set the prefixed and maybe_prefix attributes to know that
+	    fused_load_cmpi are also load insns;
+
+	4b) 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;
+
+	4c) Set the sign_extend attribute to "yes".
+
+	4d) 4a-4c 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).
+
+    5)	Add a new test case for this case.
+
+    6)	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-12   Michael Meissner  <meissner@linux.ibm.com>
 
@@ -34,6 +75,7 @@ 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/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.
@@ -41,8 +83,12 @@ gcc/
 gcc/testsuite/
 
 	* g++.target/powerpc/pr105325.C: New test.
+	* gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c: Update insn
+	counts.
 
-==================== Branch work122, patch #5 was reverted ====================
+==================== Branch work122, patch #7 was reverted ====================
+
+==================== Branch work122, patch #6 was reverted ====================
 
 ==================== Branch work122, patch #5 was reverted ====================

             reply	other threads:[~2023-06-13 17:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13 17:23 Michael Meissner [this message]
  -- strict thread matches above, loose matches on Subject: below --
2023-06-13 23:25 Michael Meissner
2023-06-13  2:58 Michael Meissner
2023-06-09 21:06 Michael Meissner
2023-06-09  6:10 Michael Meissner
2023-06-09  4:18 Michael Meissner
2023-06-09  1:33 Michael Meissner
2023-06-08 16:54 Michael Meissner
2023-06-07 20:58 Michael Meissner
2023-06-07 18:57 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=20230613172322.B2E133858D38@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).