diff mbox

[S390] Fix scheduling performance regression from my shrink-wrap patches

Message ID 87ppmji09j.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford Feb. 19, 2014, 3:35 p.m. UTC 
In the covering message for the shrink-wrap patch I said:

  Perhaps the only subtle thing is the handling of call-clobbered base
  registers.  The idea is to emit the initialising main_pool pattern in
  both early_mach -- at the very beginning of the function -- and in the
  prologue.  Then, if shrink-wrapping is used, the one added by early_mach
  will still be the first in the function.  If shrink-wrapping isn't used
  then the one added by the prologue will be the first in the function.
  s390_mainpool_start then deletes whichever isn't needed.

And as so often, the "subtle" bit wasn't really well thought out.
Having an extra unspec_volatile main_pool instruction until md_reorg
unnecessarily constrained the second scheduling pass.  This caused a
regression in some internal benchmarking.

When the base register is call-clobbered and can be set and used
anywhere in the function, there's no real need to emit it before
md_reorg.  The patch below therefore defers it until then.

Tested on s390x-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* config/s390/s390.c (s390_mainpool_start): Emit the main_pool
	instruction at the start of the function if the base register is
	call-clobbered.  Revert 2014-02-07 change.
	(s390_early_mach): Don't initialize the base register here.
	(s390_emit_prologue): Only initialize the base register if it
	is call-saved.
	(s300_extra_live_on_entry): New function.
	(TARGET_EXTRA_LIVE_ON_ENTRY): Define.

Comments

Andreas Krebbel Feb. 20, 2014, 2:47 p.m. UTC | #1 
On 19/02/14 16:35, Richard Sandiford wrote:
> gcc/
> 	* config/s390/s390.c (s390_mainpool_start): Emit the main_pool
> 	instruction at the start of the function if the base register is
> 	call-clobbered.  Revert 2014-02-07 change.
> 	(s390_early_mach): Don't initialize the base register here.
> 	(s390_emit_prologue): Only initialize the base register if it
> 	is call-saved.
> 	(s300_extra_live_on_entry): New function.
> 	(TARGET_EXTRA_LIVE_ON_ENTRY): Define.

Ok to apply. Thanks!

Bye,

-Andreas-
diff mbox

Patch

Index: gcc/config/s390/s390.c
===================================================================
--- gcc/config/s390/s390.c	2014-02-07 14:56:19.205859847 +0000
+++ gcc/config/s390/s390.c	2014-02-13 12:49:55.756293247 +0000
@@ -6668,20 +6668,21 @@  s390_mainpool_start (void)
 
   for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
     {
+      /* If the base register is call-clobbered, output the initializing
+	 instruction at the start of the function.  */
+      if (!pool->pool_insn
+	  && NONDEBUG_INSN_P (insn)
+	  && cfun->machine->base_reg
+	  && call_really_used_regs[REGNO (cfun->machine->base_reg)])
+	pool->pool_insn
+	  = emit_insn_before (gen_main_pool (cfun->machine->base_reg), insn);
+
       if (NONJUMP_INSN_P (insn)
 	  && GET_CODE (PATTERN (insn)) == SET
 	  && GET_CODE (SET_SRC (PATTERN (insn))) == UNSPEC_VOLATILE
 	  && XINT (SET_SRC (PATTERN (insn)), 1) == UNSPECV_MAIN_POOL)
 	{
-	  /* There might be two main_pool instructions if base_reg
-	     is call-clobbered; one for shrink-wrapped code and one
-	     for the rest.  We want to keep the first.  */
-	  if (pool->pool_insn)
-	    {
-	      insn = PREV_INSN (insn);
-	      delete_insn (NEXT_INSN (insn));
-	      continue;
-	    }
+	  gcc_assert (!pool->pool_insn);
 	  pool->pool_insn = insn;
 	}
 
@@ -8643,11 +8644,6 @@  s390_early_mach (void)
   /* Re-compute register info.  */
   s390_register_info ();
 
-  /* If we're using a base register, ensure that it is always valid for
-     the first non-prologue instruction.  */
-  if (cfun->machine->base_reg)
-    emit_insn_at_entry (gen_main_pool (cfun->machine->base_reg));
-
   /* Annotate all constant pool references to let the scheduler know
      they implicitly use the base register.  */
   for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
@@ -8728,7 +8724,8 @@  s390_emit_prologue (void)
 
   /* Dummy insn to mark literal pool slot.  */
 
-  if (cfun->machine->base_reg)
+  if (cfun->machine->base_reg
+      && !call_really_used_regs[REGNO (cfun->machine->base_reg)])
     emit_insn (gen_main_pool (cfun->machine->base_reg));
 
   offset = cfun_frame_layout.f0_offset;
@@ -9183,6 +9180,19 @@  s390_emit_epilogue (bool sibcall)
     }
 }
 
+/* Implement TARGET_EXTRA_LIVE_ON_ENTRY.  */
+
+static void
+s300_extra_live_on_entry (bitmap regs)
+{
+  /* If the base register is call-clobbered, the main_pool instruction
+     should go at the beginning of the function.  We only add it during
+     md_reorg.  */
+  if (cfun->machine->base_reg
+      && call_really_used_regs[REGNO (cfun->machine->base_reg)])
+    bitmap_set_bit (regs, REGNO (cfun->machine->base_reg));
+}
+
 /* Implement TARGET_SET_UP_BY_PROLOGUE.  */
 
 static void
@@ -12230,6 +12240,9 @@  #define TARGET_ATTRIBUTE_TABLE s390_attr
 #undef TARGET_CAN_INLINE_P
 #define TARGET_CAN_INLINE_P s390_can_inline_p
 
+#undef TARGET_EXTRA_LIVE_ON_ENTRY
+#define TARGET_EXTRA_LIVE_ON_ENTRY s300_extra_live_on_entry
+
 #undef TARGET_SET_UP_BY_PROLOGUE
 #define TARGET_SET_UP_BY_PROLOGUE s300_set_up_by_prologue