diff mbox series

V5, #3 of 15: Deal with prefixed instructions in rs6000_insn_cost

Message ID 20191009200316.GC2063@ibm-toto.the-meissners.org
State New
Headers show
Series V5, #3 of 15: Deal with prefixed instructions in rs6000_insn_cost | expand

Commit Message

Michael Meissner Oct. 9, 2019, 8:03 p.m. UTC 
This patch is a simplification of the previous patch to adjust the rtx insn
cost for prefixed instructions.  Rather than making it a separate function with
only one caller, I folded the code into rs6000_insn_cost.

You had asked for this to be a separate patch, so it is in this patch.

The basic problem is if there is no explicit cost predicate, rs6000_insn_cost
uses the instruction size to figure out how many instructions are present, and
make the cost a fact on that.  Since prefixed instructions are 12 bytes within
GCC (to deal with the implicit NOP), if we did not do this change, the
optimizers would try to save registers from prefixed loads because they thought
the load was more expensive.

Along with the other patches, I have done bootstraps on a little endian power8
system, and there were no regressions in the test suite.  Can I check this into
the trunk?

2019-10-08  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.c (rs6000_insn_cost): Do not make prefixed
	instructions cost more because they are larger in size.

Comments

Segher Boessenkool Oct. 11, 2019, 9:10 p.m. UTC | #1 
Hi!

On Wed, Oct 09, 2019 at 04:03:16PM -0400, Michael Meissner wrote:
> The basic problem is if there is no explicit cost predicate, rs6000_insn_cost
> uses the instruction size to figure out how many instructions are present, and
> make the cost a fact on that.  Since prefixed instructions are 12 bytes within
> GCC (to deal with the implicit NOP), if we did not do this change, the
> optimizers would try to save registers from prefixed loads because they thought
> the load was more expensive.

Maybe we should just have an attribute that says how many insns this is?
You can get rid of many prefixed_length and non_prefixed_length attributes
that way, too.

> +  int cost;
> +  int length = get_attr_length (insn);
> +  int n = length / 4;
> +
> +  /* How many real instructions are generated for this insn?  This is slightly

What is a "real" instruction?  Machine instruction?

> +     different from the length attribute, in that the length attribute counts
> +     the number of bytes.  With prefixed instructions, we don't want to count a
> +     prefixed instruction (length 12 bytes including possible NOP) as taking 3
> +     instructions, but just one.  */
> +  if (length >= 12 && get_attr_prefixed (insn) == PREFIXED_YES)
> +    {
> +      /* Single prefixed instruction.  */
> +      if (length == 12)
> +	n = 1;
> +
> +      /* A normal instruction and a prefixed instruction (16) or two back
> +	 to back prefixed instructions (20).  */
> +      else if (length == 16 || length == 20)
> +	n = 2;
> +
> +      /* Guess for larger instruction sizes.  */
> +      else
> +	n = 2 + (length - 20) / 4;

That's a pretty bad estimate.

Can you look at non_prefixed_size, will that help?


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 276715)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -20972,14 +20972,38 @@  rs6000_insn_cost (rtx_insn *insn, bool s
   if (recog_memoized (insn) < 0)
     return 0;
 
-  if (!speed)
-    return get_attr_length (insn);
+  if (speed)
+    {
+      int cost = get_attr_cost (insn);
+      if (cost > 0)
+	return cost;
+    }
 
-  int cost = get_attr_cost (insn);
-  if (cost > 0)
-    return cost;
+  int cost;
+  int length = get_attr_length (insn);
+  int n = length / 4;
+
+  /* How many real instructions are generated for this insn?  This is slightly
+     different from the length attribute, in that the length attribute counts
+     the number of bytes.  With prefixed instructions, we don't want to count a
+     prefixed instruction (length 12 bytes including possible NOP) as taking 3
+     instructions, but just one.  */
+  if (length >= 12 && get_attr_prefixed (insn) == PREFIXED_YES)
+    {
+      /* Single prefixed instruction.  */
+      if (length == 12)
+	n = 1;
+
+      /* A normal instruction and a prefixed instruction (16) or two back
+	 to back prefixed instructions (20).  */
+      else if (length == 16 || length == 20)
+	n = 2;
+
+      /* Guess for larger instruction sizes.  */
+      else
+	n = 2 + (length - 20) / 4;
+    }
 
-  int n = get_attr_length (insn) / 4;
   enum attr_type type = get_attr_type (insn);
 
   switch (type)