public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: gcc-patches@gcc.gnu.org
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Subject: [PATCH] Don't combine param and return value copies
Date: Sat, 23 May 2015 11:20:00 -0000	[thread overview]
Message-ID: <20150523083305.GB4350@bubble.grove.modra.org> (raw)

This stops combine messing with parameter and return value copies
from/to hard registers.  Bootstrapped and regression tested
powerpc64le-linux, powerpc64-linux and x86_64-linux.  In looking at a
number of different powerpc64le gcc/*.o files, I noticed a few code
generation improvements.  There were cases where a register copy was
no longer needed, cmp used in place of mr., and rlwinm instead of
rldicl.  x86_64 gcc/*.o showed no changes (apart from combine.o of
course), but should see a small improvement in compile time with this
change.

The "clear next_use when !can_combine_p" change is to fix a non-bug.
Given

1) insn defining rn
   ...
2) insn defining rn but !can_combine_p
   ...
3) insn using rn

then create_log_links might create a link from (3) to (1), I thought.
However, can_combine_p doesn't currently allow this to happen.
Obviously, any can_combine_p result depending on regno shouldn't give
a different result at (1) and (2), but there is also at test of
DF_REF_PRE_POST_MODIFY that can.  The saving grace is that pre/post
modify insns also use the register, which means next_use[rn] will
point at (2), not (3), when (1) is processed.

I came across this because at one stage I considered modifying
can_combine_p.  Someone who does so in the future might trigger the
above problem, so I thought it worth posting the change.

OK for mainline?

	* combine.c (set_return_regs): New function.
	(twiddle_first_block, twiddle_last_block): New functions.
	(create_log_links): Exclude instructions copying parameter
	values from hard regs to pseudos, and instructions copying
	return value pseudos to hard regs.  Clear next_use when
	!can_combine_p.

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	(revision 223463)
+++ gcc/combine.c	(working copy)
@@ -1048,9 +1048,79 @@ can_combine_use_p (df_ref use)
   return true;
 }
 
-/* Fill in log links field for all insns.  */
+/* Used to build set of return value regs.  Add X to the set.  */
 
 static void
+set_return_regs (rtx x, void *arg)
+{
+  HARD_REG_SET *regs = (HARD_REG_SET *) arg;
+
+  add_to_hard_reg_set (regs, GET_MODE (x), REGNO (x));
+}
+
+/* Twiddle BLOCK_FOR_INSN to TO for instructions in the first block BB
+   that we don't want to combine with other instructions.  */
+
+static void
+twiddle_first_block (basic_block bb, basic_block to)
+{
+  rtx_insn *insn;
+
+  FOR_BB_INSNS (bb, insn)
+    {
+      if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
+	break;
+      if (!NONDEBUG_INSN_P (insn))
+	continue;
+
+      /* reg,reg copies from parameter hard regs.  */
+      rtx set = single_set (insn);
+      if (set
+	  && REG_P (SET_DEST (set))
+	  && REG_P (SET_SRC (set))
+	  && HARD_REGISTER_P (SET_SRC (set)))
+	set_block_for_insn (insn, to);
+    }
+}
+
+/* Twiddle BLOCK_FOR_INSN to TO for instructions in the last block BB
+   that we don't want to combine with other instructions.  */
+
+static void
+twiddle_last_block (basic_block bb, basic_block to, HARD_REG_SET return_regs)
+{
+  rtx_insn *insn;
+
+  FOR_BB_INSNS_REVERSE (bb, insn)
+    {
+      if (CALL_P (insn))
+	break;
+      if (!NONDEBUG_INSN_P (insn))
+	continue;
+
+      rtx reg = NULL_RTX;
+      /* use_return_regster added USEs.  */
+      if (GET_CODE (PATTERN (insn)) == USE)
+	reg = XEXP (PATTERN (insn), 0);
+      else
+	{
+	  /* reg,reg copies that set return value hard regs.  */
+	  rtx set = single_set (insn);
+	  if (set && REG_P (SET_SRC (set)))
+	    reg = SET_DEST (set);
+	}
+      if (reg
+	  && REG_P (reg)
+	  && HARD_REGISTER_P (reg)
+	  && overlaps_hard_reg_set_p (return_regs,
+				      GET_MODE (reg), REGNO (reg)))
+	set_block_for_insn (insn, to);
+    }
+}
+
+/* Fill in log links field for all insns that we wish to combine.  */
+
+static void
 create_log_links (void)
 {
   basic_block bb;
@@ -1057,9 +1127,28 @@ create_log_links (void)
   rtx_insn **next_use;
   rtx_insn *insn;
   df_ref def, use;
+  HARD_REG_SET return_regs;
 
   next_use = XCNEWVEC (rtx_insn *, max_reg_num ());
 
+  /* Don't combine instructions copying parameter values from hard
+     regs to pseudos.  Exclude such instructions from LOG_LINKS by
+     temporarily zapping BLOCK_FOR_INSN.  */
+
+  bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+  twiddle_first_block (bb, 0);
+
+  /* Similarly, don't combine instructions copying return values
+     from pseudos to hard regs.  */
+
+  CLEAR_HARD_REG_SET (return_regs);
+  diddle_return_value (set_return_regs, &return_regs);
+  if (!hard_reg_set_empty_p (return_regs))
+    {
+      bb = EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb;
+      twiddle_last_block (bb, 0, return_regs);
+    }
+
   /* Pass through each block from the end, recording the uses of each
      register and establishing log links when def is encountered.
      Note that we do not clear next_use array in order to save time,
@@ -1087,12 +1176,12 @@ create_log_links (void)
               if (!next_use[regno])
                 continue;
 
+	      use_insn = next_use[regno];
+	      next_use[regno] = NULL;
+
 	      if (!can_combine_def_p (def))
 		continue;
 
-	      use_insn = next_use[regno];
-	      next_use[regno] = NULL;
-
 	      if (BLOCK_FOR_INSN (use_insn) != bb)
 		continue;
 
@@ -1103,7 +1192,7 @@ create_log_links (void)
 		 we might wind up changing the semantics of the insn,
 		 even if reload can make what appear to be valid
 		 assignments later.  */
-	      if (regno < FIRST_PSEUDO_REGISTER
+	      if (HARD_REGISTER_NUM_P (regno)
 		  && asm_noperands (PATTERN (use_insn)) >= 0)
 		continue;
 
@@ -1124,6 +1213,16 @@ create_log_links (void)
         }
     }
 
+  /* Repair BLOCK_FOR_INSN.  */
+
+  bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+  twiddle_first_block (bb, bb);
+  if (!hard_reg_set_empty_p (return_regs))
+    {
+      bb = EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb;
+      twiddle_last_block (bb, bb, return_regs);
+    }
+
   free (next_use);
 }
 

-- 
Alan Modra
Australia Development Lab, IBM

             reply	other threads:[~2015-05-23  8:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-23 11:20 Alan Modra [this message]
2015-05-23 11:20 ` Andrew Pinski
2015-05-23 19:40 ` Segher Boessenkool
2015-05-25  5:24   ` Alan Modra
2015-05-25  7:30     ` Alan Modra
2015-05-25 20:19     ` Segher Boessenkool
2015-05-26  8:15       ` Alan Modra
2015-05-28 15:33         ` Segher Boessenkool

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=20150523083305.GB4350@bubble.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=segher@kernel.crashing.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).