public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFA] Avoid unnecessary load-immediate in coremark
@ 2022-09-27 19:53 Jeff Law
  2022-09-29  7:44 ` Richard Biener
  2022-09-30 10:47 ` Richard Sandiford
  0 siblings, 2 replies; 5+ messages in thread
From: Jeff Law @ 2022-09-27 19:53 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]


This is another minor improvement to coremark.   I suspect this only 
improves code size as the load-immediate was likely issuing with the ret 
statement on multi-issue machines.


Basically we're failing to utilize conditional equivalences during the 
post-reload CSE pass.  So if a particular block is only reached when a 
certain condition holds (say for example a4 == 0) and the block has an 
assignment like a4 = 0, we would fail to eliminate the unnecessary 
assignment.


So the way this works, as we enter each block in reload_cse_regs_1 we 
look at the block's predecessors to see if all of them have the same 
implicit assignment.  If they do, then we create a dummy insn 
representing that implicit assignment.


Before processing the first real insn, we enter the implicit assignment 
into the cselib hash tables.    This deferred action is necessary 
because of CODE_LABEL handling in cselib -- when it sees a CODE_LABEL it 
wipes state.  So we have to add the implicit assignment after processing 
the (optional) CODE_LABEL, but before processing real insns.


Note we have to walk all the block's predecessors to verify they all 
have the same implicit assignment.  That could potentially be expensive, 
so we limit it to cases where there are only a few predecessors.   For 
reference on x86_64, 81% of the cases where implicit assignments can be 
found are for single predecessor blocks.  96% have two preds, 99.1% have 
3 preds, 99.6% have 4 preds, 99.8% have 5 preds and so-on.   While there 
were cases where all 19 preds had the same implicit assignment capturing 
those cases just doesn't seem terribly important.   I put the clamp at 3 
preds.    If folks think it's important, I could certainly make that a 
PARAM.


Bootstrapped and regression tested on x86.  Bootstrapped on riscv as well.


OK for the trunk?


Jeff



[-- Attachment #2: P --]
[-- Type: text/plain, Size: 4771 bytes --]

gcc/
	* postreload.cc (reload_cse_regs_1): Record implicit sets from
	conditional branches into the cselib tables.

gcc/testsuite/

	* gcc.target/riscv/implict-set.c: New test.

	
diff --git a/gcc/postreload.cc b/gcc/postreload.cc
index 41f61d32648..2f155a239ae 100644
--- a/gcc/postreload.cc
+++ b/gcc/postreload.cc
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "emit-rtl.h"
 #include "recog.h"
 
+#include "cfghooks.h"
 #include "cfgrtl.h"
 #include "cfgbuild.h"
 #include "cfgcleanup.h"
@@ -221,13 +222,108 @@ reload_cse_regs_1 (void)
   init_alias_analysis ();
 
   FOR_EACH_BB_FN (bb, cfun)
-    FOR_BB_INSNS (bb, insn)
-      {
-	if (INSN_P (insn))
-	  cfg_changed |= reload_cse_simplify (insn, testreg);
+    {
+      /* If BB has a small number of predecessors, see if each of the
+	 has the same implicit set.  If so, record that implicit set so
+	 that we can add it to the cselib tables.  */
+      rtx_insn *implicit_set;
 
-	cselib_process_insn (insn);
-      }
+      implicit_set = NULL;
+      if (EDGE_COUNT (bb->preds) <= 3)
+	{
+	  edge e;
+	  edge_iterator ei;
+	  rtx src = NULL_RTX;
+	  rtx dest = NULL_RTX;
+	  bool found = true;
+
+	  /* Iterate over each incoming edge and see if they
+	     all have the same implicit set.  */
+	  FOR_EACH_EDGE (e, ei, bb->preds)
+	    {
+	      /* If the predecessor does not end in a conditional
+		 jump, then it does not have an implicit set.  */
+	      if (e->src != ENTRY_BLOCK_PTR_FOR_FN (cfun)
+		  && !block_ends_with_condjump_p (e->src))
+		{
+		  found = false;
+		  break;
+		}
+
+	      /* We know the predecessor ends with a conditional
+		 jump.  Now dig into the actal form of the jump
+		 to potentially extract an implicit set.  */
+	      rtx_insn *condjump = BB_END (e->src);
+	      if (condjump
+		  && any_condjump_p (condjump)
+		  && onlyjump_p (condjump))
+		{
+		  /* Extract the condition.  */
+		  rtx pat = PATTERN (condjump);
+		  rtx i_t_e = SET_SRC (pat);
+		  gcc_assert (GET_CODE (i_t_e) == IF_THEN_ELSE);
+		  rtx cond = XEXP (i_t_e, 0);
+		  if ((GET_CODE (cond) == EQ
+		       && GET_CODE (XEXP (i_t_e, 1)) == LABEL_REF
+		       && XEXP (XEXP (i_t_e, 1), 0) == BB_HEAD (bb))
+		      || (GET_CODE (cond) == NE
+			  && XEXP (i_t_e, 2) == pc_rtx
+			  && e->src->next_bb == bb))
+		    {
+		      /* If this is the first time through record
+			 the source and destination.  */
+		      if (!dest)
+			{
+			  dest = XEXP (cond, 0);
+			  src = XEXP (cond, 1);
+			}
+		      /* If this is not the first time through, then
+			 verify the source and destination match.  */
+		      else if (dest == XEXP (cond, 0) && src == XEXP (cond, 1))
+			;
+		      else
+			{
+			  found = false;
+			  break;
+			}
+		    }
+		}
+	      else
+		{
+		  found = false;
+		  break;
+		}
+	    }
+
+	  /* If all the incoming edges had the same implicit
+	     set, then create a dummy insn for that set.
+
+	     It will be entered into the cselib tables before
+	     we process the first real insn in this block.  */
+	  if (dest && found)
+	    implicit_set = make_insn_raw (gen_rtx_SET (dest, src));
+	}
+
+      FOR_BB_INSNS (bb, insn)
+	{
+	  if (INSN_P (insn))
+	    {
+	      /* If we recorded an implicit set, enter it
+		 into the tables before the first real insn.
+
+		 We have to do it this way because a CODE_LABEL
+		 will flush the cselib tables.  */
+	      if (implicit_set)
+		{
+		  cselib_process_insn (implicit_set);
+		  implicit_set = NULL;
+		}
+	      cfg_changed |= reload_cse_simplify (insn, testreg);
+	    }
+
+	  cselib_process_insn (insn);
+	}
+    }
 
   /* Clean up.  */
   end_alias_analysis ();
diff --git a/gcc/testsuite/gcc.target/riscv/implicit-set.c b/gcc/testsuite/gcc.target/riscv/implicit-set.c
new file mode 100644
index 00000000000..91106bb5d80
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/implicit-set.c
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -dp" } */
+/* This was extracted from coremark.  */
+
+
+typedef signed short ee_s16;
+typedef struct list_data_s
+{
+    ee_s16 data16;
+    ee_s16 idx;
+} list_data;
+
+typedef struct list_head_s
+{
+    struct list_head_s *next;
+    struct list_data_s *info;
+} list_head;
+
+
+list_head *
+core_list_find(list_head *list, list_data *info)
+{
+    if (info->idx >= 0)
+    {
+        while (list && (list->info->idx != info->idx))
+            list = list->next;
+        return list;
+    }
+    else
+    {
+        while (list && ((list->info->data16 & 0xff) != info->data16))
+            list = list->next;
+        return list;
+    }
+}
+
+/* There was an unnecessary assignment to the return value until
+   recently.  Scan for that in the resulting output.  */
+/* { dg-final { scan-assembler-not "li\\ta0,0" } } */
+

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-01 19:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27 19:53 [RFA] Avoid unnecessary load-immediate in coremark Jeff Law
2022-09-29  7:44 ` Richard Biener
2022-10-01 18:58   ` Jeff Law
2022-09-30 10:47 ` Richard Sandiford
2022-10-01 19:03   ` Jeff Law

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).