public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: gcc-patches@gcc.gnu.org
Cc: hubicka@ucw.cz, ubizjak@gmail.com
Subject: [x86] Cache result of expensive_function_p between frame layouts
Date: Mon, 30 Sep 2019 14:11:00 -0000	[thread overview]
Message-ID: <mptimp998ng.fsf@arm.com> (raw)

ix86_compute_frame_layout sets use_fast_prologue_epilogue if
the function isn't more expensive than a certain threshold,
where the threshold depends on the number of saved registers.
However, the RA is allowed to insert and delete instructions
as it goes along, which can change whether this threshold is
crossed or not.

I hit this with an RA change I'm working on.  Rematerialisation
was able to remove an instruction and avoid a spill, which happened
to bring the size of the function below the threshold.  But since
nothing legitimately frame-related had changed, there was no need for
the RA to lay out the frame again.  We then failed the final sanity
check in lra_eliminate.

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

Richard


2019-09-30  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/i386/i386.h (ix86_frame::expensive_p): New field.
	(ix86_frame::expensive_count): Likewise.
	* config/i386/i386.c (ix86_compute_frame_layout): Make the choice
	of use_fast_prologue_epilogue robust against incidental changes
	in function size.

Index: gcc/config/i386/i386.h
===================================================================
--- gcc/config/i386/i386.h	2019-09-26 08:37:44.000000000 +0100
+++ gcc/config/i386/i386.h	2019-09-30 15:07:51.784114465 +0100
@@ -2643,6 +2643,11 @@ struct GTY(()) ix86_frame
   /* When save_regs_using_mov is set, emit prologue using
      move instead of push instructions.  */
   bool save_regs_using_mov;
+
+  /* Assume without checking that:
+       EXPENSIVE_P = expensive_function_p (EXPENSIVE_COUNT).  */
+  bool expensive_p;
+  int expensive_count;
 };
 
 /* Machine specific frame tracking during prologue/epilogue generation.  All
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	2019-09-26 08:37:44.000000000 +0100
+++ gcc/config/i386/i386.c	2019-09-30 15:07:51.784114465 +0100
@@ -5876,7 +5876,14 @@ ix86_compute_frame_layout (void)
 	 case function is known to be outside hot spot (this is known with
 	 feedback only).  Weight the size of function by number of registers
 	 to save as it is cheap to use one or two push instructions but very
-	 slow to use many of them.  */
+	 slow to use many of them.
+
+	 Calling this hook multiple times with the same frame requirements
+	 must produce the same layout, since the RA might otherwise be
+	 unable to reach a fixed point or might fail its final sanity checks.
+	 This means that once we've assumed that a function does or doesn't
+	 have a particular size, we have to stick to that assumption
+	 regardless of how the function has changed since.  */
       if (count)
 	count = (count - 1) * FAST_PROLOGUE_INSN_COUNT;
       if (node->frequency < NODE_FREQUENCY_NORMAL
@@ -5884,8 +5891,14 @@ ix86_compute_frame_layout (void)
 	      && node->frequency < NODE_FREQUENCY_HOT))
 	m->use_fast_prologue_epilogue = false;
       else
-	m->use_fast_prologue_epilogue
-	   = !expensive_function_p (count);
+	{
+	  if (count != frame->expensive_count)
+	    {
+	      frame->expensive_count = count;
+	      frame->expensive_p = expensive_function_p (count);
+	    }
+	  m->use_fast_prologue_epilogue = !frame->expensive_p;
+	}
     }
 
   frame->save_regs_using_mov

             reply	other threads:[~2019-09-30 14:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-30 14:11 Richard Sandiford [this message]
2019-09-30 17:57 ` Jan Hubicka

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=mptimp998ng.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=ubizjak@gmail.com \
    /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).