[PATCH 0/8] [RS6000] rs6000_rtx_costs V2

[PATCH 0/8] [RS6000] rs6000_rtx_costs V2

[Alan Modra]

This is a revised version of the patch series posted at
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553919.html

Patch 8/8 in particular changes rather a lot more than the original
!speed changes.  Bootstrapped and regression tested powerpc64le-linux
and powerpc64-linux biarch.

Alan Modra (8):
  [RS6000] rs6000_rtx_costs comment
  [RS6000] rs6000_rtx_costs for AND
  [RS6000] rs6000_rtx_costs tidy AND
  [RS6000] rs6000_rtx_costs tidy break/return
  [RS6000] rs6000_rtx_costs cost IOR
  [RS6000] rs6000_rtx_costs multi-insn constants
  [RS6000] rs6000_rtx_costs reduce cost for SETs
  [RS6000] rs6000_rtx_costs for !speed

 gcc/config/rs6000/rs6000.c | 516 +++++++++++++++++++++++++------------
 gcc/config/rs6000/rs6000.h |  23 --
 2 files changed, 350 insertions(+), 189 deletions(-)

[PATCH 1/8] [RS6000] rs6000_rtx_costs comment

[Alan Modra]

This lays out the ground rules for following patches.

	* config/rs6000/rs6000.c (rs6000_rtx_costs): Expand comment.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b58eeae2b98..97180bb3819 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21208,7 +21208,46 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
 
 /* Compute a (partial) cost for rtx X.  Return true if the complete
    cost has been computed, and false if subexpressions should be
-   scanned.  In either case, *TOTAL contains the cost result.  */
+   scanned.  In either case, *TOTAL contains the cost result.
+
+   1) Calls from places like optabs.c:avoid_expensive_constant will
+   come here with OUTER_CODE set to an operation such as AND with X
+   being a CONST_INT or other CONSTANT_P type.  This will be compared
+   against set_src_cost, where we'll come here with OUTER_CODE as SET
+   and X the same constant.
+
+   2) Calls from places like combine.c:distribute_and_simplify_rtx are
+   asking whether a possibly quite complex SET_SRC can be implemented
+   more cheaply than some other logically equivalent SET_SRC.
+
+   3) Calls from places like ifcvt.c:default_noce_conversion_profitable_p
+   will come here via seq_cost which calls set_rtx_cost on single set
+   insns.  set_rtx_cost passes the pattern of a SET insn in X with
+   OUTER_CODE as INSN.  The overall cost should be comparable to
+   rs6000_insn_cost since the code is comparing one insn sequence
+   (some of which may be costed by insn_cost) against another sequence.
+   Note the difference between set_rtx_cost, which costs the entire
+   SET, and set_src_cost, which costs just the SET_SRC.
+
+   4) Calls from places like cprop.c:try_replace_reg will also come
+   here via set_rtx_cost, with X either a valid pattern of a SET or
+   one where some registers have been replaced with constants.  The
+   replacements may make the SET invalid, for example if
+     (set (reg1) (and (reg2) (const_int 0xfff)))
+   has reg2 replaced as
+     (set (reg1) (and (symbol_ref) (const_int 0xfff)))
+   then the replacement can't be implemented in one instruction and
+   really the cost should be higher by one instruction.  However,
+   the cost for invalid insns doesn't matter much except that a
+   higher cost may lead to their rejection earlier.
+
+   5) fwprop.c:should_replace_address puts yet another wrinkle on this
+   function, where we prefer an address calculation that is more
+   complex yet has the same address_cost.  In this case "more
+   complex" is determined by having a higher set_src_cost.  So for
+   example, if we want a plain (reg) address to be replaced with
+   (plus (reg) (const)) when possible then PLUS needs to cost more
+   than zero here.  */
 
 static bool
 rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,

[PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Alan Modra]

The existing "case AND" in this function is not sufficient for
optabs.c:avoid_expensive_constant usage, where the AND is passed in
outer_code.  We'd like to cost AND of rs6000_is_valid_and_mask
or rs6000_is_valid_2insn_and variety there, so that those masks aren't
seen as expensive (ie. better to load to a reg then AND).

	* config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT
	AND handling with IOR/XOR.  Move costing for AND with
	rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to
	CONST_INT.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 97180bb3819..e870ba0039a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21264,16 +21264,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	    || outer_code == MINUS)
 	   && (satisfies_constraint_I (x)
 	       || satisfies_constraint_L (x)))
-	  || (outer_code == AND
-	      && (satisfies_constraint_K (x)
-		  || (mode == SImode
-		      ? satisfies_constraint_L (x)
-		      : satisfies_constraint_J (x))))
-	  || ((outer_code == IOR || outer_code == XOR)
+	  || ((outer_code == AND || outer_code == IOR || outer_code == XOR)
 	      && (satisfies_constraint_K (x)
 		  || (mode == SImode
 		      ? satisfies_constraint_L (x)
 		      : satisfies_constraint_J (x))))
+	  || (outer_code == AND
+	      && rs6000_is_valid_and_mask (x, mode))
 	  || outer_code == ASHIFT
 	  || outer_code == ASHIFTRT
 	  || outer_code == LSHIFTRT
@@ -21310,7 +21307,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 		    || outer_code == IOR
 		    || outer_code == XOR)
 		   && (INTVAL (x)
-		       & ~ (unsigned HOST_WIDE_INT) 0xffffffff) == 0))
+		       & ~ (unsigned HOST_WIDE_INT) 0xffffffff) == 0)
+	       || (outer_code == AND
+		   && rs6000_is_valid_2insn_and (x, mode)))
 	{
 	  *total = COSTS_N_INSNS (1);
 	  return true;
@@ -21448,26 +21447,6 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	      *total += COSTS_N_INSNS (1);
 	      return true;
 	    }
-
-	  /* rotate-and-mask (no rotate), andi., andis.: 1 insn.  */
-	  HOST_WIDE_INT val = INTVAL (XEXP (x, 1));
-	  if (rs6000_is_valid_and_mask (XEXP (x, 1), mode)
-	      || (val & 0xffff) == val
-	      || (val & 0xffff0000) == val
-	      || ((val & 0xffff) == 0 && mode == SImode))
-	    {
-	      *total = rtx_cost (left, mode, AND, 0, speed);
-	      *total += COSTS_N_INSNS (1);
-	      return true;
-	    }
-
-	  /* 2 insns.  */
-	  if (rs6000_is_valid_2insn_and (XEXP (x, 1), mode))
-	    {
-	      *total = rtx_cost (left, mode, AND, 0, speed);
-	      *total += COSTS_N_INSNS (2);
-	      return true;
-	    }
 	}
 
       *total = COSTS_N_INSNS (1);

[PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND

[Alan Modra]

	* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy AND code.
	Don't avoid recursion on const_int shift count.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e870ba0039a..bc5e51aa5ce 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21253,6 +21253,7 @@ static bool
 rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
 {
+  rtx right;
   int code = GET_CODE (x);
 
   switch (code)
@@ -21430,7 +21431,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
       return false;
 
     case AND:
-      if (CONST_INT_P (XEXP (x, 1)))
+      *total = COSTS_N_INSNS (1);
+      right = XEXP (x, 1);
+      if (CONST_INT_P (right))
 	{
 	  rtx left = XEXP (x, 0);
 	  rtx_code left_code = GET_CODE (left);
@@ -21439,17 +21442,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  if ((left_code == ROTATE
 	       || left_code == ASHIFT
 	       || left_code == LSHIFTRT)
-	      && rs6000_is_valid_shift_mask (XEXP (x, 1), left, mode))
+	      && rs6000_is_valid_shift_mask (right, left, mode))
 	    {
-	      *total = rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
-	      if (!CONST_INT_P (XEXP (left, 1)))
-		*total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
-	      *total += COSTS_N_INSNS (1);
+	      *total += rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
+	      *total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
 	      return true;
 	    }
 	}
-
-      *total = COSTS_N_INSNS (1);
       return false;
 
     case IOR:

[PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return

[Alan Modra]

Most cases use "return false" rather than breaking out of the switch.
Do so in all cases.

	* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy break/return.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index bc5e51aa5ce..383d2901c9f 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21371,7 +21371,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	*total = rs6000_cost->fp;
       else
 	*total = rs6000_cost->dmul;
-      break;
+      return false;
 
     case DIV:
     case MOD:
@@ -21539,7 +21539,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  *total = rs6000_cost->fp;
 	  return false;
 	}
-      break;
+      return false;
 
     case NE:
     case EQ:
@@ -21577,13 +21577,11 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  *total = 0;
 	  return true;
 	}
-      break;
+      return false;
 
     default:
-      break;
+      return false;
     }
-
-  return false;
 }
 
 /* Debug form of r6000_rtx_costs that is selected if -mdebug=cost.  */

[PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR

[Alan Modra]

	* config/rs6000/rs6000.c (rotate_insert_cost): New function.
	(rs6000_rtx_costs): Cost IOR.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 383d2901c9f..15a806fe307 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
 	 && get_attr_cannot_copy (insn);
 }
 
+/* Handle rtx_costs for scalar integer rotate and insert insns.  */
+
+static bool
+rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
+		    int *total)
+{
+  if (GET_CODE (right) == AND
+      && CONST_INT_P (XEXP (right, 1))
+      && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)
+    {
+      rtx leftop = XEXP (left, 0);
+      rtx rightop = XEXP (right, 0);
+
+      /* rotlsi3_insert_5.  */
+      if (REG_P (leftop)
+	  && REG_P (rightop)
+	  && mode == SImode
+	  && UINTVAL (XEXP (left, 1)) != 0
+	  && UINTVAL (XEXP (right, 1)) != 0
+	  && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
+	return true;
+      /* rotldi3_insert_6.  */
+      if (REG_P (leftop)
+	  && REG_P (rightop)
+	  && mode == DImode
+	  && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
+	return true;
+      /* rotldi3_insert_7.  */
+      if (REG_P (leftop)
+	  && REG_P (rightop)
+	  && mode == DImode
+	  && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
+	return true;
+
+      rtx mask = 0;
+      rtx shift = leftop;
+      rtx_code shift_code = GET_CODE (shift);
+      /* rotl<mode>3_insert.  */
+      if (shift_code == ROTATE
+	  || shift_code == ASHIFT
+	  || shift_code == LSHIFTRT)
+	mask = right;
+      else
+	{
+	  shift = rightop;
+	  shift_code = GET_CODE (shift);
+	  /* rotl<mode>3_insert_2.  */
+	  if (shift_code == ROTATE
+	      || shift_code == ASHIFT
+	      || shift_code == LSHIFTRT)
+	    mask = left;
+	}
+      if (mask
+	  && CONST_INT_P (XEXP (shift, 1))
+	  && rs6000_is_valid_insert_mask (XEXP (mask, 1), shift, mode))
+	{
+	  *total += rtx_cost (XEXP (shift, 0), mode, shift_code, 0, speed);
+	  *total += rtx_cost (XEXP (mask, 0), mode, AND, 0, speed);
+	  return true;
+	}
+    }
+  /* rotl<mode>3_insert_3.  */
+  if (GET_CODE (right) == ASHIFT
+      && CONST_INT_P (XEXP (right, 1))
+      && (INTVAL (XEXP (right, 1))
+	  == exact_log2 (UINTVAL (XEXP (left, 1)) + 1)))
+    {
+      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
+      *total += rtx_cost (XEXP (right, 0), mode, ASHIFT, 0, speed);
+      return true;
+    }
+  /* rotl<mode>3_insert_4.  */
+  if (GET_CODE (right) == LSHIFTRT
+      && CONST_INT_P (XEXP (right, 1))
+      && mode == SImode
+      && (INTVAL (XEXP (right, 1))
+	  + exact_log2 (-UINTVAL (XEXP (left, 1)))) == 32)
+    {
+      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
+      *total += rtx_cost (XEXP (right, 0), mode, LSHIFTRT, 0, speed);
+      return true;
+    }
+  return false;
+}
+
 /* Compute a (partial) cost for rtx X.  Return true if the complete
    cost has been computed, and false if subexpressions should be
    scanned.  In either case, *TOTAL contains the cost result.
@@ -21253,7 +21338,7 @@ static bool
 rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
 {
-  rtx right;
+  rtx left, right;
   int code = GET_CODE (x);
 
   switch (code)
@@ -21435,7 +21520,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
       right = XEXP (x, 1);
       if (CONST_INT_P (right))
 	{
-	  rtx left = XEXP (x, 0);
+	  left = XEXP (x, 0);
 	  rtx_code left_code = GET_CODE (left);
 
 	  /* rotate-and-mask: 1 insn.  */
@@ -21452,9 +21537,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
       return false;
 
     case IOR:
-      /* FIXME */
       *total = COSTS_N_INSNS (1);
-      return true;
+      left = XEXP (x, 0);
+      if (GET_CODE (left) == AND
+	  && CONST_INT_P (XEXP (left, 1)))
+	{
+	  right = XEXP (x, 1);
+	  if (rotate_insert_cost (left, right, mode, speed, total))
+	    return true;
+	}
+      return false;
 
     case CLZ:
     case XOR:

[PATCH 6/8] [RS6000] rs6000_rtx_costs multi-insn constants

[Alan Modra]

This small patch to rs6000_rtx_costs considerably improves code
generated for large constants in 64-bit code, teaching gcc that it is
better to load a constant from memory than to generate a sequence of
up to five dependent instructions.  Note that the rs6000 backend does
generate large constants as loads from memory at expand time but
optimisation passes replace them with SETs of the value due to not
having correct costs.

	PR 94393
	* config/rs6000/rs6000.c (rs6000_rtx_costs): Cost multi-insn
	constants.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 15a806fe307..76aedbfae6f 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21343,7 +21343,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 
   switch (code)
     {
-      /* On the RS/6000, if it is valid in the insn, it is free.  */
+      /* (reg) is costed at zero by rtlanal.c:rtx_cost.  That sets a
+	 baseline for rtx costs:  If a constant is valid in an insn,
+	 it is free.  */
     case CONST_INT:
       if (((outer_code == SET
 	    || outer_code == PLUS
@@ -21404,6 +21406,17 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 
     case CONST_DOUBLE:
     case CONST_WIDE_INT:
+      /* Subtract one insn here for consistency with the above code
+	 that returns one less than the actual number of insns for
+	 SETs.  Don't subtract one for other than SETs, because other
+	 operations will require the constant to be loaded to a
+	 register before performing the operation.  All special cases
+	 for codes other than SET must be handled before we get
+	 here.  */
+      *total = COSTS_N_INSNS (num_insns_constant (x, mode)
+			      - (outer_code == SET ? 1 : 0));
+      return true;
+
     case CONST:
     case HIGH:
     case SYMBOL_REF:

[PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs

[Alan Modra]

The aim of this patch is to make rtx_costs for SETs closer to
insn_cost for SETs.  One visible effect on powerpc code is increased
if-conversion.

	* config/rs6000/rs6000.c (rs6000_rtx_costs): Reduce cost of SET
	operands.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 76aedbfae6f..d455aa52427 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21684,6 +21684,35 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	}
       return false;
 
+    case SET:
+      /* On entry the value in *TOTAL is the number of general purpose
+	 regs being set, multiplied by COSTS_N_INSNS (1).  Handle
+	 costing of set operands specially since in most cases we have
+	 an instruction rather than just a piece of RTL and should
+	 return a cost comparable to insn_cost.  That's a little
+	 complicated because in some cases the cost of SET operands is
+	 non-zero, see point 5 above and cost of PLUS for example, and
+	 in others it is zero, for example for (set (reg) (reg)).
+	 But (set (reg) (reg)) has the same insn_cost as
+	 (set (reg) (plus (reg) (reg))).  Hack around this by
+	 subtracting COSTS_N_INSNS (1) from the operand cost in cases
+	 were we add at least COSTS_N_INSNS (1) for some operation.
+	 However, don't do so for constants.  Constants might cost
+	 more than zero when they require more than one instruction,
+	 and we do want the cost of extra instructions.  */
+      {
+	rtx_code src_code = GET_CODE (SET_SRC (x));
+	if (src_code == CONST_INT
+	    || src_code == CONST_DOUBLE
+	    || src_code == CONST_WIDE_INT)
+	  return false;
+	int set_cost = (rtx_cost (SET_SRC (x), mode, SET, 1, speed)
+			+ rtx_cost (SET_DEST (x), mode, SET, 0, speed));
+	if (set_cost >= COSTS_N_INSNS (1))
+	  *total += set_cost - COSTS_N_INSNS (1);
+	return true;
+      }
+
     default:
       return false;
     }

[PATCH 8/8] [RS6000] rs6000_rtx_costs for !speed

[Alan Modra]

When optimizing for size we shouldn't be using metrics based on speed
or vice-versa.  rtlanal.c:get_full_rtx_cost wants both speed and size
metric from rs6000_rtx_costs independent of the global optimize_size.

Note that the patch changes param_simultaneous_prefetches,
param_l1_cache_size, param_l1_cache_line_size and param_l2_cache_size,
which were previously all set to zero for optimize_size.  I think that
was a bug.  Those params are a function of the processor.

	* config/rs6000/rs6000.h (rs6000_cost): Don't declare.
	(struct processor_costs): Move to..
	* config/rs6000/rs6000.c: ..here.
	(rs6000_cost): Make static.
	(rs6000_option_override_internal): Ignore optimize_size when
	setting up rs6000_cost.
	(rs6000_insn_cost): Take into account optimize_size here
	instead.
	(rs6000_emit_parity): Likewise.
	(rs6000_rtx_costs): Don't use rs6000_cost when !speed.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d455aa52427..14ecbad5df4 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -497,7 +497,26 @@ rs6000_store_data_bypass_p (rtx_insn *out_insn, rtx_insn *in_insn)
 \f
 /* Processor costs (relative to an add) */
 
-const struct processor_costs *rs6000_cost;
+struct processor_costs {
+  const int mulsi;	  /* cost of SImode multiplication.  */
+  const int mulsi_const;  /* cost of SImode multiplication by constant.  */
+  const int mulsi_const9; /* cost of SImode mult by short constant.  */
+  const int muldi;	  /* cost of DImode multiplication.  */
+  const int divsi;	  /* cost of SImode division.  */
+  const int divdi;	  /* cost of DImode division.  */
+  const int fp;		  /* cost of simple SFmode and DFmode insns.  */
+  const int dmul;	  /* cost of DFmode multiplication (and fmadd).  */
+  const int sdiv;	  /* cost of SFmode division (fdivs).  */
+  const int ddiv;	  /* cost of DFmode division (fdiv).  */
+  const int cache_line_size;    /* cache line size in bytes. */
+  const int l1_cache_size;	/* size of l1 cache, in kilobytes.  */
+  const int l2_cache_size;	/* size of l2 cache, in kilobytes.  */
+  const int simultaneous_prefetches; /* number of parallel prefetch
+					operations.  */
+  const int sfdf_convert;	/* cost of SF->DF conversion.  */
+};
+
+static const struct processor_costs *rs6000_cost;
 
 /* Instruction size costs on 32bit processors.  */
 static const
@@ -4618,131 +4637,128 @@ rs6000_option_override_internal (bool global_init_p)
     }
 
   /* Initialize rs6000_cost with the appropriate target costs.  */
-  if (optimize_size)
-    rs6000_cost = TARGET_POWERPC64 ? &size64_cost : &size32_cost;
-  else
-    switch (rs6000_tune)
-      {
-      case PROCESSOR_RS64A:
-	rs6000_cost = &rs64a_cost;
-	break;
+  switch (rs6000_tune)
+    {
+    case PROCESSOR_RS64A:
+      rs6000_cost = &rs64a_cost;
+      break;
 
-      case PROCESSOR_MPCCORE:
-	rs6000_cost = &mpccore_cost;
-	break;
+    case PROCESSOR_MPCCORE:
+      rs6000_cost = &mpccore_cost;
+      break;
 
-      case PROCESSOR_PPC403:
-	rs6000_cost = &ppc403_cost;
-	break;
+    case PROCESSOR_PPC403:
+      rs6000_cost = &ppc403_cost;
+      break;
 
-      case PROCESSOR_PPC405:
-	rs6000_cost = &ppc405_cost;
-	break;
+    case PROCESSOR_PPC405:
+      rs6000_cost = &ppc405_cost;
+      break;
 
-      case PROCESSOR_PPC440:
-	rs6000_cost = &ppc440_cost;
-	break;
+    case PROCESSOR_PPC440:
+      rs6000_cost = &ppc440_cost;
+      break;
 
-      case PROCESSOR_PPC476:
-	rs6000_cost = &ppc476_cost;
-	break;
+    case PROCESSOR_PPC476:
+      rs6000_cost = &ppc476_cost;
+      break;
 
-      case PROCESSOR_PPC601:
-	rs6000_cost = &ppc601_cost;
-	break;
+    case PROCESSOR_PPC601:
+      rs6000_cost = &ppc601_cost;
+      break;
 
-      case PROCESSOR_PPC603:
-	rs6000_cost = &ppc603_cost;
-	break;
+    case PROCESSOR_PPC603:
+      rs6000_cost = &ppc603_cost;
+      break;
 
-      case PROCESSOR_PPC604:
-	rs6000_cost = &ppc604_cost;
-	break;
+    case PROCESSOR_PPC604:
+      rs6000_cost = &ppc604_cost;
+      break;
 
-      case PROCESSOR_PPC604e:
-	rs6000_cost = &ppc604e_cost;
-	break;
+    case PROCESSOR_PPC604e:
+      rs6000_cost = &ppc604e_cost;
+      break;
 
-      case PROCESSOR_PPC620:
-	rs6000_cost = &ppc620_cost;
-	break;
+    case PROCESSOR_PPC620:
+      rs6000_cost = &ppc620_cost;
+      break;
 
-      case PROCESSOR_PPC630:
-	rs6000_cost = &ppc630_cost;
-	break;
+    case PROCESSOR_PPC630:
+      rs6000_cost = &ppc630_cost;
+      break;
 
-      case PROCESSOR_CELL:
-	rs6000_cost = &ppccell_cost;
-	break;
+    case PROCESSOR_CELL:
+      rs6000_cost = &ppccell_cost;
+      break;
 
-      case PROCESSOR_PPC750:
-      case PROCESSOR_PPC7400:
-	rs6000_cost = &ppc750_cost;
-	break;
+    case PROCESSOR_PPC750:
+    case PROCESSOR_PPC7400:
+      rs6000_cost = &ppc750_cost;
+      break;
 
-      case PROCESSOR_PPC7450:
-	rs6000_cost = &ppc7450_cost;
-	break;
+    case PROCESSOR_PPC7450:
+      rs6000_cost = &ppc7450_cost;
+      break;
 
-      case PROCESSOR_PPC8540:
-      case PROCESSOR_PPC8548:
-	rs6000_cost = &ppc8540_cost;
-	break;
+    case PROCESSOR_PPC8540:
+    case PROCESSOR_PPC8548:
+      rs6000_cost = &ppc8540_cost;
+      break;
 
-      case PROCESSOR_PPCE300C2:
-      case PROCESSOR_PPCE300C3:
-	rs6000_cost = &ppce300c2c3_cost;
-	break;
+    case PROCESSOR_PPCE300C2:
+    case PROCESSOR_PPCE300C3:
+      rs6000_cost = &ppce300c2c3_cost;
+      break;
 
-      case PROCESSOR_PPCE500MC:
-	rs6000_cost = &ppce500mc_cost;
-	break;
+    case PROCESSOR_PPCE500MC:
+      rs6000_cost = &ppce500mc_cost;
+      break;
 
-      case PROCESSOR_PPCE500MC64:
-	rs6000_cost = &ppce500mc64_cost;
-	break;
+    case PROCESSOR_PPCE500MC64:
+      rs6000_cost = &ppce500mc64_cost;
+      break;
 
-      case PROCESSOR_PPCE5500:
-	rs6000_cost = &ppce5500_cost;
-	break;
+    case PROCESSOR_PPCE5500:
+      rs6000_cost = &ppce5500_cost;
+      break;
 
-      case PROCESSOR_PPCE6500:
-	rs6000_cost = &ppce6500_cost;
-	break;
+    case PROCESSOR_PPCE6500:
+      rs6000_cost = &ppce6500_cost;
+      break;
 
-      case PROCESSOR_TITAN:
-	rs6000_cost = &titan_cost;
-	break;
+    case PROCESSOR_TITAN:
+      rs6000_cost = &titan_cost;
+      break;
 
-      case PROCESSOR_POWER4:
-      case PROCESSOR_POWER5:
-	rs6000_cost = &power4_cost;
-	break;
+    case PROCESSOR_POWER4:
+    case PROCESSOR_POWER5:
+      rs6000_cost = &power4_cost;
+      break;
 
-      case PROCESSOR_POWER6:
-	rs6000_cost = &power6_cost;
-	break;
+    case PROCESSOR_POWER6:
+      rs6000_cost = &power6_cost;
+      break;
 
-      case PROCESSOR_POWER7:
-	rs6000_cost = &power7_cost;
-	break;
+    case PROCESSOR_POWER7:
+      rs6000_cost = &power7_cost;
+      break;
 
-      case PROCESSOR_POWER8:
-	rs6000_cost = &power8_cost;
-	break;
+    case PROCESSOR_POWER8:
+      rs6000_cost = &power8_cost;
+      break;
 
-      case PROCESSOR_POWER9:
-      case PROCESSOR_POWER10:
-	rs6000_cost = &power9_cost;
-	break;
+    case PROCESSOR_POWER9:
+    case PROCESSOR_POWER10:
+      rs6000_cost = &power9_cost;
+      break;
 
-      case PROCESSOR_PPCA2:
-	rs6000_cost = &ppca2_cost;
-	break;
+    case PROCESSOR_PPCA2:
+      rs6000_cost = &ppca2_cost;
+      break;
 
-      default:
-	gcc_unreachable ();
-      }
+    default:
+      gcc_unreachable ();
+    }
 
   if (global_init_p)
     {
@@ -21438,15 +21454,17 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 
     case PLUS:
     case MINUS:
-      if (FLOAT_MODE_P (mode))
+      if (speed && FLOAT_MODE_P (mode))
 	*total = rs6000_cost->fp;
       else
 	*total = COSTS_N_INSNS (1);
       return false;
 
     case MULT:
-      if (CONST_INT_P (XEXP (x, 1))
-	  && satisfies_constraint_I (XEXP (x, 1)))
+      if (!speed)
+	*total = COSTS_N_INSNS (1);
+      else if (CONST_INT_P (XEXP (x, 1))
+	       && satisfies_constraint_I (XEXP (x, 1)))
 	{
 	  if (INTVAL (XEXP (x, 1)) >= -256
 	      && INTVAL (XEXP (x, 1)) <= 255)
@@ -21465,7 +21483,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
       return false;
 
     case FMA:
-      if (mode == SFmode)
+      if (!speed)
+	*total = COSTS_N_INSNS (1);
+      else if (mode == SFmode)
 	*total = rs6000_cost->fp;
       else
 	*total = rs6000_cost->dmul;
@@ -21475,8 +21495,10 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
     case MOD:
       if (FLOAT_MODE_P (mode))
 	{
-	  *total = mode == DFmode ? rs6000_cost->ddiv
-				  : rs6000_cost->sdiv;
+	  if (!speed)
+	    *total = COSTS_N_INSNS (1);
+	  else
+	    *total = mode == DFmode ? rs6000_cost->ddiv : rs6000_cost->sdiv;
 	  return false;
 	}
       /* FALLTHRU */
@@ -21495,7 +21517,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	}
       else
 	{
-	  if (GET_MODE (XEXP (x, 1)) == DImode)
+	  if (!speed)
+	    *total = COSTS_N_INSNS (1);
+	  else if (GET_MODE (XEXP (x, 1)) == DImode)
 	    *total = rs6000_cost->divdi;
 	  else
 	    *total = rs6000_cost->divsi;
@@ -21587,7 +21611,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
       if (outer_code == TRUNCATE
 	  && GET_CODE (XEXP (x, 0)) == MULT)
 	{
-	  if (mode == DImode)
+	  if (!speed)
+	    *total = COSTS_N_INSNS (1);
+	  else if (mode == DImode)
 	    *total = rs6000_cost->muldi;
 	  else
 	    *total = rs6000_cost->mulsi;
@@ -21622,11 +21648,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
     case FIX:
     case UNSIGNED_FIX:
     case FLOAT_TRUNCATE:
-      *total = rs6000_cost->fp;
+      if (!speed)
+	*total = COSTS_N_INSNS (1);
+      else
+	*total = rs6000_cost->fp;
       return false;
 
     case FLOAT_EXTEND:
-      if (mode == DFmode)
+      if (!speed)
+	*total = COSTS_N_INSNS (1);
+      else if (mode == DFmode)
 	*total = rs6000_cost->sfdf_convert;
       else
 	*total = rs6000_cost->fp;
@@ -21773,6 +21804,10 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
       n = length / 4;
     }
 
+  const struct processor_costs *proc_cost = rs6000_cost;
+  if (optimize_size)
+    proc_cost = TARGET_POWERPC64 ? &size64_cost : &size32_cost;
+
   enum attr_type type = get_attr_type (insn);
 
   switch (type)
@@ -21787,16 +21822,16 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
       switch (get_attr_size (insn))
 	{
 	case SIZE_8:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const9;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi_const9;
 	  break;
 	case SIZE_16:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi_const;
 	  break;
 	case SIZE_32:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi;
 	  break;
 	case SIZE_64:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->muldi;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->muldi;
 	  break;
 	default:
 	  gcc_unreachable ();
@@ -21806,10 +21841,10 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
       switch (get_attr_size (insn))
 	{
 	case SIZE_32:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->divsi;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->divsi;
 	  break;
 	case SIZE_64:
-	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->divdi;
+	  cost = COSTS_N_INSNS (n - 1) + proc_cost->divdi;
 	  break;
 	default:
 	  gcc_unreachable ();
@@ -21817,16 +21852,16 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
       break;
 
     case TYPE_FP:
-      cost = n * rs6000_cost->fp;
+      cost = n * proc_cost->fp;
       break;
     case TYPE_DMUL:
-      cost = n * rs6000_cost->dmul;
+      cost = n * proc_cost->dmul;
       break;
     case TYPE_SDIV:
-      cost = n * rs6000_cost->sdiv;
+      cost = n * proc_cost->sdiv;
       break;
     case TYPE_DDIV:
-      cost = n * rs6000_cost->ddiv;
+      cost = n * proc_cost->ddiv;
       break;
 
     case TYPE_SYNC:
@@ -22388,7 +22423,7 @@ rs6000_emit_parity (rtx dst, rtx src)
   if (mode == SImode)
     {
       /* Is mult+shift >= shift+xor+shift+xor?  */
-      if (rs6000_cost->mulsi_const >= COSTS_N_INSNS (3))
+      if (!optimize_size && rs6000_cost->mulsi_const >= COSTS_N_INSNS (3))
 	{
 	  rtx tmp1, tmp2, tmp3, tmp4;
 
@@ -22411,7 +22446,7 @@ rs6000_emit_parity (rtx dst, rtx src)
   else
     {
       /* Is mult+shift >= shift+xor+shift+xor+shift+xor?  */
-      if (rs6000_cost->muldi >= COSTS_N_INSNS (5))
+      if (!optimize_size && rs6000_cost->muldi >= COSTS_N_INSNS (5))
 	{
 	  rtx tmp1, tmp2, tmp3, tmp4, tmp5, tmp6;
 
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index bbd8060e143..9daf55adc02 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -1869,29 +1869,6 @@ extern scalar_int_mode rs6000_pmode;
 #define REVERSE_CONDITION(CODE, MODE) rs6000_reverse_condition (MODE, CODE)
 
 \f
-/* Target cpu costs.  */
-
-struct processor_costs {
-  const int mulsi;	  /* cost of SImode multiplication.  */
-  const int mulsi_const;  /* cost of SImode multiplication by constant.  */
-  const int mulsi_const9; /* cost of SImode mult by short constant.  */
-  const int muldi;	  /* cost of DImode multiplication.  */
-  const int divsi;	  /* cost of SImode division.  */
-  const int divdi;	  /* cost of DImode division.  */
-  const int fp;		  /* cost of simple SFmode and DFmode insns.  */
-  const int dmul;	  /* cost of DFmode multiplication (and fmadd).  */
-  const int sdiv;	  /* cost of SFmode division (fdivs).  */
-  const int ddiv;	  /* cost of DFmode division (fdiv).  */
-  const int cache_line_size;    /* cache line size in bytes. */
-  const int l1_cache_size;	/* size of l1 cache, in kilobytes.  */
-  const int l2_cache_size;	/* size of l2 cache, in kilobytes.  */
-  const int simultaneous_prefetches; /* number of parallel prefetch
-					operations.  */
-  const int sfdf_convert;	/* cost of SF->DF conversion.  */
-};
-
-extern const struct processor_costs *rs6000_cost;
-\f
 /* Control the assembler format that we output.  */
 
 /* A C string constant describing how to begin a comment in the target

Re: [PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs

[will schmidt]

On Thu, 2020-10-08 at 09:27 +1030, Alan Modra via Gcc-patches wrote:
> The aim of this patch is to make rtx_costs for SETs closer to
> insn_cost for SETs.  One visible effect on powerpc code is increased
> if-conversion.
> 
> 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Reduce cost of SET
> 	operands.
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 76aedbfae6f..d455aa52427 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -21684,6 +21684,35 @@ rs6000_rtx_costs (rtx x, machine_mode mode,
> int outer_code,
>  	}
>        return false;
> 
> +    case SET:
> +      /* On entry the value in *TOTAL is the number of general
> purpose
> +	 regs being set, multiplied by COSTS_N_INSNS (1).  Handle
> +	 costing of set operands specially since in most cases we have
> +	 an instruction rather than just a piece of RTL and should
> +	 return a cost comparable to insn_cost.  That's a little
> +	 complicated because in some cases the cost of SET operands is
> +	 non-zero, see point 5 above and cost of PLUS for example, and
> +	 in others it is zero, for example for (set (reg) (reg)).
> +	 But (set (reg) (reg)) has the same insn_cost as
> +	 (set (reg) (plus (reg) (reg))).  Hack around this by
> +	 subtracting COSTS_N_INSNS (1) from the operand cost in cases
> +	 were we add at least COSTS_N_INSNS (1) for some operation.

s/were/where/   :-)

> +	 However, don't do so for constants.  Constants might cost
> +	 more than zero when they require more than one instruction,
> +	 and we do want the cost of extra instructions.  */
> +      {
> +	rtx_code src_code = GET_CODE (SET_SRC (x));
> +	if (src_code == CONST_INT
> +	    || src_code == CONST_DOUBLE
> +	    || src_code == CONST_WIDE_INT)
> +	  return false;
> +	int set_cost = (rtx_cost (SET_SRC (x), mode, SET, 1, speed)
> +			+ rtx_cost (SET_DEST (x), mode, SET, 0,
> speed));
> +	if (set_cost >= COSTS_N_INSNS (1))
> +	  *total += set_cost - COSTS_N_INSNS (1);
> +	return true;
> +      }
> +
>      default:
>        return false;
>      }

lgtm, 
thanks
-Will

Re: [PATCH 1/8] [RS6000] rs6000_rtx_costs comment

[Segher Boessenkool]

Hi!

On Thu, Oct 08, 2020 at 09:27:53AM +1030, Alan Modra wrote:
> This lays out the ground rules for following patches.
> 
> 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Expand comment.

This is okay for trunk.  Thanks!


Segher

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Segher Boessenkool]

On Thu, Oct 08, 2020 at 09:27:54AM +1030, Alan Modra wrote:
> The existing "case AND" in this function is not sufficient for
> optabs.c:avoid_expensive_constant usage, where the AND is passed in
> outer_code.  We'd like to cost AND of rs6000_is_valid_and_mask
> or rs6000_is_valid_2insn_and variety there, so that those masks aren't
> seen as expensive (ie. better to load to a reg then AND).
> 
> 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT
> 	AND handling with IOR/XOR.  Move costing for AND with
> 	rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to
> 	CONST_INT.

Sorry this took so long to review :-(

On 64-bit BE this leads to *bigger* code, and closer observation shows
that some common sequences degrade on all configs.  This seems to mostly
be about "andc" (and its dot form).  It wasn't costed properly before,
but after your patch, a single instruction is replaced by three.

Could you look into this?


Segher

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Alan Modra]

On Tue, Oct 20, 2020 at 01:55:56PM -0500, Segher Boessenkool wrote:
> On Thu, Oct 08, 2020 at 09:27:54AM +1030, Alan Modra wrote:
> > The existing "case AND" in this function is not sufficient for
> > optabs.c:avoid_expensive_constant usage, where the AND is passed in
> > outer_code.  We'd like to cost AND of rs6000_is_valid_and_mask
> > or rs6000_is_valid_2insn_and variety there, so that those masks aren't
> > seen as expensive (ie. better to load to a reg then AND).
> > 
> > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT
> > 	AND handling with IOR/XOR.  Move costing for AND with
> > 	rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to
> > 	CONST_INT.
> 
> Sorry this took so long to review :-(
> 
> On 64-bit BE this leads to *bigger* code, and closer observation shows
> that some common sequences degrade on all configs.  This seems to mostly
> be about "andc" (and its dot form).  It wasn't costed properly before,
> but after your patch, a single instruction is replaced by three.
> 
> Could you look into this?

~/build/gcc-alan/gcc$ for z in *.o; do if test `objdump -dr $z | grep andc | wc -l` != `objdump -dr ../../gcc/gcc/$z | grep andc | wc -l`; then echo $z; fi; done
gimplify.o
insn-emit.o
insn-opinit.o
insn-recog.o
rs6000-string.o

All of these are exactly the case I talked about in
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553919.html

"Sometimes correct insn cost leads to unexpected results.  For
example:

extern unsigned bar (void);
unsigned
f1 (unsigned a)
{
  if ((a & 0x01000200) == 0x01000200)
    return bar ();
  return 0;
}

emits for a & 0x01000200
 (set (reg) (and (reg) (const_int 0x01000200)))
at expand time (two rlwinm insns) rather than the older
 (set (reg) (const_int 0x01000200))
 (set (reg) (and (reg) (reg)))
which is three insns.  However, since 0x01000200 is needed later the
older code after optimisation is smaller."

Things have changed slightly since I wrote the above, with the two
rlwinm insns being emitted at expand time, so you see
 (set (reg) (and (reg) (const_int 0xffffffffff0003ff)))
 (set (reg) (and (reg) (const_int 0x0000000001fffe00)))
but of course that doesn't change anything regarding the cost of
"a & 0x01000200".

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Segher Boessenkool]

On Wed, Oct 21, 2020 at 01:27:42PM +1030, Alan Modra wrote:
> On Tue, Oct 20, 2020 at 01:55:56PM -0500, Segher Boessenkool wrote:
> > On Thu, Oct 08, 2020 at 09:27:54AM +1030, Alan Modra wrote:
> > > The existing "case AND" in this function is not sufficient for
> > > optabs.c:avoid_expensive_constant usage, where the AND is passed in
> > > outer_code.  We'd like to cost AND of rs6000_is_valid_and_mask
> > > or rs6000_is_valid_2insn_and variety there, so that those masks aren't
> > > seen as expensive (ie. better to load to a reg then AND).
> > > 
> > > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT
> > > 	AND handling with IOR/XOR.  Move costing for AND with
> > > 	rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to
> > > 	CONST_INT.
> > 
> > Sorry this took so long to review :-(
> > 
> > On 64-bit BE this leads to *bigger* code, and closer observation shows
> > that some common sequences degrade on all configs.  This seems to mostly
> > be about "andc" (and its dot form).  It wasn't costed properly before,
> > but after your patch, a single instruction is replaced by three.
> > 
> > Could you look into this?
> 
> ~/build/gcc-alan/gcc$ for z in *.o; do if test `objdump -dr $z | grep andc | wc -l` != `objdump -dr ../../gcc/gcc/$z | grep andc | wc -l`; then echo $z; fi; done
> gimplify.o
> insn-emit.o
> insn-opinit.o
> insn-recog.o
> rs6000-string.o
> 
> All of these are exactly the case I talked about in
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553919.html

For a kernel build (my testcase) it happens more often.

> "Sometimes correct insn cost leads to unexpected results.  For
> example:
> 
> extern unsigned bar (void);
> unsigned
> f1 (unsigned a)
> {
>   if ((a & 0x01000200) == 0x01000200)
>     return bar ();
>   return 0;
> }
> 
> emits for a & 0x01000200
>  (set (reg) (and (reg) (const_int 0x01000200)))
> at expand time (two rlwinm insns) rather than the older
>  (set (reg) (const_int 0x01000200))
>  (set (reg) (and (reg) (reg)))

And that is bad.  Why on earth does expand "optimise" this?  It should
not, it hinders various *real* optimisations!

> which is three insns.  However, since 0x01000200 is needed later the
> older code after optimisation is smaller."
> 
> Things have changed slightly since I wrote the above, with the two
> rlwinm insns being emitted at expand time, so you see
>  (set (reg) (and (reg) (const_int 0xffffffffff0003ff)))
>  (set (reg) (and (reg) (const_int 0x0000000001fffe00)))

It has done that for many years?

> but of course that doesn't change anything regarding the cost of
> "a & 0x01000200".

Yeah.  But the problem is that cost that are "better", "closer to
reality", sometimes result in worse results :-(

Anyway:

+              || (outer_code == AND
+                  && rs6000_is_valid_2insn_and (x, mode)))
        {
          *total = COSTS_N_INSNS (1);
          return true;

It should return COSTS_N_INSNS (2) for that?

Testing with that now.


Segher

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Alan Modra]

On Wed, Oct 21, 2020 at 03:29:11PM -0500, Segher Boessenkool wrote:
> Anyway:
> 
> +              || (outer_code == AND
> +                  && rs6000_is_valid_2insn_and (x, mode)))
>         {
>           *total = COSTS_N_INSNS (1);
>           return true;
> 
> It should return COSTS_N_INSNS (2) for that?

No, it should not!

      /* (reg) is costed at zero by rtlanal.c:rtx_cost.  That sets a
	 baseline for rtx costs:  If a constant is valid in an insn,
	 it is free.  */

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Segher Boessenkool]

On Thu, Oct 22, 2020 at 08:41:50AM +1030, Alan Modra wrote:
> On Wed, Oct 21, 2020 at 03:29:11PM -0500, Segher Boessenkool wrote:
> > Anyway:
> > 
> > +              || (outer_code == AND
> > +                  && rs6000_is_valid_2insn_and (x, mode)))
> >         {
> >           *total = COSTS_N_INSNS (1);
> >           return true;
> > 
> > It should return COSTS_N_INSNS (2) for that?
> 
> No, it should not!
> 
>       /* (reg) is costed at zero by rtlanal.c:rtx_cost.  That sets a
> 	 baseline for rtx costs:  If a constant is valid in an insn,
> 	 it is free.  */

Yeah, this keeps tripping me up.

The patch is okay for trunk (as I told you offline).  Thanks!

The regressions are really from existing bad behaviour in expand :-(


Segher

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Hans-Peter Nilsson]

On Thu, 22 Oct 2020, Alan Modra via Gcc-patches wrote:

Hi!

> On Wed, Oct 21, 2020 at 03:29:11PM -0500, Segher Boessenkool wrote:
> > Anyway:
> >
> > +              || (outer_code == AND
> > +                  && rs6000_is_valid_2insn_and (x, mode)))
> >         {
> >           *total = COSTS_N_INSNS (1);
> >           return true;
> >
> > It should return COSTS_N_INSNS (2) for that?
>
> No, it should not!
>
>       /* (reg) is costed at zero by rtlanal.c:rtx_cost.  That sets a
> 	 baseline for rtx costs:  If a constant is valid in an insn,
> 	 it is free.  */

From where is this quote?  My "git grep" fails to find it for me
(on master).  It seems like a port-specific commment so I'd
have expected to find it somewhere in config/rs6000.

brgds, H-P

Re: [PATCH 2/8] [RS6000] rs6000_rtx_costs for AND

[Alan Modra]

On Fri, Oct 23, 2020 at 07:18:44PM -0400, Hans-Peter Nilsson wrote:
> On Thu, 22 Oct 2020, Alan Modra via Gcc-patches wrote:
> >       /* (reg) is costed at zero by rtlanal.c:rtx_cost.  That sets a
> > 	 baseline for rtx costs:  If a constant is valid in an insn,
> > 	 it is free.  */
> 
> >From where is this quote?  My "git grep" fails to find it for me
> (on master).  It seems like a port-specific commment so I'd
> have expected to find it somewhere in config/rs6000.

It's in a patch in the rs6000_rtx_costs series that hasn't yet been
committed.
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555757.html

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/8] [RS6000] rs6000_rtx_costs V2

[Alan Modra]

Hi Segher,
I've been holding off pinging these knowing you had a lot of other
review work, but maybe that's settling down now?  You already OK'd
1/8, 2/8 and 6/8.

[PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555754.html

[PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555755.html

[PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555756.html

[PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555758.html

[PATCH 8/8] [RS6000] rs6000_rtx_costs for !speed
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555759.html

[RS6000] rotate and mask constants
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555760.html

[RS6000] Adjust testcases for power10 instructions V3
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557587.html

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/8] [RS6000] rs6000_rtx_costs V2

[Alan Modra]

On Sat, Dec 05, 2020 at 07:42:07PM +1030, Alan Modra wrote:
> Hi Segher,
> I've been holding off pinging these knowing you had a lot of other
> review work, but maybe that's settling down now?  You already OK'd
> 1/8, 2/8 and 6/8.

Ping.

> [PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555754.html
> 
> [PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555755.html
> 
> [PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555756.html
> 
> [PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555758.html
> 
> [PATCH 8/8] [RS6000] rs6000_rtx_costs for !speed
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555759.html
> 
> [RS6000] rotate and mask constants
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555760.html
> 
> [RS6000] Adjust testcases for power10 instructions V3
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557587.html

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND

[Alan Modra]

Ping.

On Tue, Jan 12, 2021 at 02:01:57PM +1030, Alan Modra wrote:
> Ping
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555754.html
> 
> On Thu, Oct 08, 2020 at 09:27:55AM +1030, Alan Modra wrote:
> > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy AND code.
> > 	Don't avoid recursion on const_int shift count.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index e870ba0039a..bc5e51aa5ce 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21253,6 +21253,7 @@ static bool
> >  rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
> >  {
> > +  rtx right;
> >    int code = GET_CODE (x);
> >  
> >    switch (code)
> > @@ -21430,7 +21431,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >        return false;
> >  
> >      case AND:
> > -      if (CONST_INT_P (XEXP (x, 1)))
> > +      *total = COSTS_N_INSNS (1);
> > +      right = XEXP (x, 1);
> > +      if (CONST_INT_P (right))
> >  	{
> >  	  rtx left = XEXP (x, 0);
> >  	  rtx_code left_code = GET_CODE (left);
> > @@ -21439,17 +21442,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	  if ((left_code == ROTATE
> >  	       || left_code == ASHIFT
> >  	       || left_code == LSHIFTRT)
> > -	      && rs6000_is_valid_shift_mask (XEXP (x, 1), left, mode))
> > +	      && rs6000_is_valid_shift_mask (right, left, mode))
> >  	    {
> > -	      *total = rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
> > -	      if (!CONST_INT_P (XEXP (left, 1)))
> > -		*total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
> > -	      *total += COSTS_N_INSNS (1);
> > +	      *total += rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
> > +	      *total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
> >  	      return true;
> >  	    }
> >  	}
> > -
> > -      *total = COSTS_N_INSNS (1);
> >        return false;
> >  
> >      case IOR:

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return

[Alan Modra]

Ping.

On Tue, Jan 12, 2021 at 02:02:09PM +1030, Alan Modra wrote:
> Ping
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555755.html
> 
> On Thu, Oct 08, 2020 at 09:27:56AM +1030, Alan Modra wrote:
> > Most cases use "return false" rather than breaking out of the switch.
> > Do so in all cases.
> > 
> > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy break/return.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index bc5e51aa5ce..383d2901c9f 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21371,7 +21371,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	*total = rs6000_cost->fp;
> >        else
> >  	*total = rs6000_cost->dmul;
> > -      break;
> > +      return false;
> >  
> >      case DIV:
> >      case MOD:
> > @@ -21539,7 +21539,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	  *total = rs6000_cost->fp;
> >  	  return false;
> >  	}
> > -      break;
> > +      return false;
> >  
> >      case NE:
> >      case EQ:
> > @@ -21577,13 +21577,11 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	  *total = 0;
> >  	  return true;
> >  	}
> > -      break;
> > +      return false;
> >  
> >      default:
> > -      break;
> > +      return false;
> >      }
> > -
> > -  return false;
> >  }
> >  
> >  /* Debug form of r6000_rtx_costs that is selected if -mdebug=cost.  */

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR

[Alan Modra]

Ping.

On Tue, Jan 12, 2021 at 02:02:18PM +1030, Alan Modra wrote:
> Ping
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555756.html
> 
> On Thu, Oct 08, 2020 at 09:27:57AM +1030, Alan Modra wrote:
> > 	* config/rs6000/rs6000.c (rotate_insert_cost): New function.
> > 	(rs6000_rtx_costs): Cost IOR.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 383d2901c9f..15a806fe307 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
> >  	 && get_attr_cannot_copy (insn);
> >  }
> >  
> > +/* Handle rtx_costs for scalar integer rotate and insert insns.  */
> > +
> > +static bool
> > +rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
> > +		    int *total)
> > +{
> > +  if (GET_CODE (right) == AND
> > +      && CONST_INT_P (XEXP (right, 1))
> > +      && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)
> > +    {
> > +      rtx leftop = XEXP (left, 0);
> > +      rtx rightop = XEXP (right, 0);
> > +
> > +      /* rotlsi3_insert_5.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == SImode
> > +	  && UINTVAL (XEXP (left, 1)) != 0
> > +	  && UINTVAL (XEXP (right, 1)) != 0
> > +	  && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
> > +	return true;
> > +      /* rotldi3_insert_6.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == DImode
> > +	  && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
> > +	return true;
> > +      /* rotldi3_insert_7.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == DImode
> > +	  && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
> > +	return true;
> > +
> > +      rtx mask = 0;
> > +      rtx shift = leftop;
> > +      rtx_code shift_code = GET_CODE (shift);
> > +      /* rotl<mode>3_insert.  */
> > +      if (shift_code == ROTATE
> > +	  || shift_code == ASHIFT
> > +	  || shift_code == LSHIFTRT)
> > +	mask = right;
> > +      else
> > +	{
> > +	  shift = rightop;
> > +	  shift_code = GET_CODE (shift);
> > +	  /* rotl<mode>3_insert_2.  */
> > +	  if (shift_code == ROTATE
> > +	      || shift_code == ASHIFT
> > +	      || shift_code == LSHIFTRT)
> > +	    mask = left;
> > +	}
> > +      if (mask
> > +	  && CONST_INT_P (XEXP (shift, 1))
> > +	  && rs6000_is_valid_insert_mask (XEXP (mask, 1), shift, mode))
> > +	{
> > +	  *total += rtx_cost (XEXP (shift, 0), mode, shift_code, 0, speed);
> > +	  *total += rtx_cost (XEXP (mask, 0), mode, AND, 0, speed);
> > +	  return true;
> > +	}
> > +    }
> > +  /* rotl<mode>3_insert_3.  */
> > +  if (GET_CODE (right) == ASHIFT
> > +      && CONST_INT_P (XEXP (right, 1))
> > +      && (INTVAL (XEXP (right, 1))
> > +	  == exact_log2 (UINTVAL (XEXP (left, 1)) + 1)))
> > +    {
> > +      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
> > +      *total += rtx_cost (XEXP (right, 0), mode, ASHIFT, 0, speed);
> > +      return true;
> > +    }
> > +  /* rotl<mode>3_insert_4.  */
> > +  if (GET_CODE (right) == LSHIFTRT
> > +      && CONST_INT_P (XEXP (right, 1))
> > +      && mode == SImode
> > +      && (INTVAL (XEXP (right, 1))
> > +	  + exact_log2 (-UINTVAL (XEXP (left, 1)))) == 32)
> > +    {
> > +      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
> > +      *total += rtx_cost (XEXP (right, 0), mode, LSHIFTRT, 0, speed);
> > +      return true;
> > +    }
> > +  return false;
> > +}
> > +
> >  /* Compute a (partial) cost for rtx X.  Return true if the complete
> >     cost has been computed, and false if subexpressions should be
> >     scanned.  In either case, *TOTAL contains the cost result.
> > @@ -21253,7 +21338,7 @@ static bool
> >  rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
> >  {
> > -  rtx right;
> > +  rtx left, right;
> >    int code = GET_CODE (x);
> >  
> >    switch (code)
> > @@ -21435,7 +21520,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >        right = XEXP (x, 1);
> >        if (CONST_INT_P (right))
> >  	{
> > -	  rtx left = XEXP (x, 0);
> > +	  left = XEXP (x, 0);
> >  	  rtx_code left_code = GET_CODE (left);
> >  
> >  	  /* rotate-and-mask: 1 insn.  */
> > @@ -21452,9 +21537,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >        return false;
> >  
> >      case IOR:
> > -      /* FIXME */
> >        *total = COSTS_N_INSNS (1);
> > -      return true;
> > +      left = XEXP (x, 0);
> > +      if (GET_CODE (left) == AND
> > +	  && CONST_INT_P (XEXP (left, 1)))
> > +	{
> > +	  right = XEXP (x, 1);
> > +	  if (rotate_insert_cost (left, right, mode, speed, total))
> > +	    return true;
> > +	}
> > +      return false;
> >  
> >      case CLZ:
> >      case XOR:

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs

[Alan Modra]

Ping.

On Tue, Jan 12, 2021 at 02:02:27PM +1030, Alan Modra wrote:
> Ping
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555758.html
> 
> On Thu, Oct 08, 2020 at 09:27:59AM +1030, Alan Modra wrote:
> > The aim of this patch is to make rtx_costs for SETs closer to
> > insn_cost for SETs.  One visible effect on powerpc code is increased
> > if-conversion.
> > 
> > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Reduce cost of SET
> > 	operands.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 76aedbfae6f..d455aa52427 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21684,6 +21684,35 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	}
> >        return false;
> >  
> > +    case SET:
> > +      /* On entry the value in *TOTAL is the number of general purpose
> > +	 regs being set, multiplied by COSTS_N_INSNS (1).  Handle
> > +	 costing of set operands specially since in most cases we have
> > +	 an instruction rather than just a piece of RTL and should
> > +	 return a cost comparable to insn_cost.  That's a little
> > +	 complicated because in some cases the cost of SET operands is
> > +	 non-zero, see point 5 above and cost of PLUS for example, and
> > +	 in others it is zero, for example for (set (reg) (reg)).
> > +	 But (set (reg) (reg)) has the same insn_cost as
> > +	 (set (reg) (plus (reg) (reg))).  Hack around this by
> > +	 subtracting COSTS_N_INSNS (1) from the operand cost in cases
> > +	 were we add at least COSTS_N_INSNS (1) for some operation.
> > +	 However, don't do so for constants.  Constants might cost
> > +	 more than zero when they require more than one instruction,
> > +	 and we do want the cost of extra instructions.  */
> > +      {
> > +	rtx_code src_code = GET_CODE (SET_SRC (x));
> > +	if (src_code == CONST_INT
> > +	    || src_code == CONST_DOUBLE
> > +	    || src_code == CONST_WIDE_INT)
> > +	  return false;
> > +	int set_cost = (rtx_cost (SET_SRC (x), mode, SET, 1, speed)
> > +			+ rtx_cost (SET_DEST (x), mode, SET, 0, speed));
> > +	if (set_cost >= COSTS_N_INSNS (1))
> > +	  *total += set_cost - COSTS_N_INSNS (1);
> > +	return true;
> > +      }
> > +
> >      default:
> >        return false;
> >      }

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 8/8] [RS6000] rs6000_rtx_costs for !speed

[Alan Modra]

Ping.

On Tue, Jan 12, 2021 at 02:02:36PM +1030, Alan Modra wrote:
> Ping
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555759.html
> 
> On Thu, Oct 08, 2020 at 09:28:00AM +1030, Alan Modra wrote:
> > When optimizing for size we shouldn't be using metrics based on speed
> > or vice-versa.  rtlanal.c:get_full_rtx_cost wants both speed and size
> > metric from rs6000_rtx_costs independent of the global optimize_size.
> > 
> > Note that the patch changes param_simultaneous_prefetches,
> > param_l1_cache_size, param_l1_cache_line_size and param_l2_cache_size,
> > which were previously all set to zero for optimize_size.  I think that
> > was a bug.  Those params are a function of the processor.
> > 
> > 	* config/rs6000/rs6000.h (rs6000_cost): Don't declare.
> > 	(struct processor_costs): Move to..
> > 	* config/rs6000/rs6000.c: ..here.
> > 	(rs6000_cost): Make static.
> > 	(rs6000_option_override_internal): Ignore optimize_size when
> > 	setting up rs6000_cost.
> > 	(rs6000_insn_cost): Take into account optimize_size here
> > 	instead.
> > 	(rs6000_emit_parity): Likewise.
> > 	(rs6000_rtx_costs): Don't use rs6000_cost when !speed.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index d455aa52427..14ecbad5df4 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -497,7 +497,26 @@ rs6000_store_data_bypass_p (rtx_insn *out_insn, rtx_insn *in_insn)
> >  \f
> >  /* Processor costs (relative to an add) */
> >  
> > -const struct processor_costs *rs6000_cost;
> > +struct processor_costs {
> > +  const int mulsi;	  /* cost of SImode multiplication.  */
> > +  const int mulsi_const;  /* cost of SImode multiplication by constant.  */
> > +  const int mulsi_const9; /* cost of SImode mult by short constant.  */
> > +  const int muldi;	  /* cost of DImode multiplication.  */
> > +  const int divsi;	  /* cost of SImode division.  */
> > +  const int divdi;	  /* cost of DImode division.  */
> > +  const int fp;		  /* cost of simple SFmode and DFmode insns.  */
> > +  const int dmul;	  /* cost of DFmode multiplication (and fmadd).  */
> > +  const int sdiv;	  /* cost of SFmode division (fdivs).  */
> > +  const int ddiv;	  /* cost of DFmode division (fdiv).  */
> > +  const int cache_line_size;    /* cache line size in bytes. */
> > +  const int l1_cache_size;	/* size of l1 cache, in kilobytes.  */
> > +  const int l2_cache_size;	/* size of l2 cache, in kilobytes.  */
> > +  const int simultaneous_prefetches; /* number of parallel prefetch
> > +					operations.  */
> > +  const int sfdf_convert;	/* cost of SF->DF conversion.  */
> > +};
> > +
> > +static const struct processor_costs *rs6000_cost;
> >  
> >  /* Instruction size costs on 32bit processors.  */
> >  static const
> > @@ -4618,131 +4637,128 @@ rs6000_option_override_internal (bool global_init_p)
> >      }
> >  
> >    /* Initialize rs6000_cost with the appropriate target costs.  */
> > -  if (optimize_size)
> > -    rs6000_cost = TARGET_POWERPC64 ? &size64_cost : &size32_cost;
> > -  else
> > -    switch (rs6000_tune)
> > -      {
> > -      case PROCESSOR_RS64A:
> > -	rs6000_cost = &rs64a_cost;
> > -	break;
> > +  switch (rs6000_tune)
> > +    {
> > +    case PROCESSOR_RS64A:
> > +      rs6000_cost = &rs64a_cost;
> > +      break;
> >  
> > -      case PROCESSOR_MPCCORE:
> > -	rs6000_cost = &mpccore_cost;
> > -	break;
> > +    case PROCESSOR_MPCCORE:
> > +      rs6000_cost = &mpccore_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC403:
> > -	rs6000_cost = &ppc403_cost;
> > -	break;
> > +    case PROCESSOR_PPC403:
> > +      rs6000_cost = &ppc403_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC405:
> > -	rs6000_cost = &ppc405_cost;
> > -	break;
> > +    case PROCESSOR_PPC405:
> > +      rs6000_cost = &ppc405_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC440:
> > -	rs6000_cost = &ppc440_cost;
> > -	break;
> > +    case PROCESSOR_PPC440:
> > +      rs6000_cost = &ppc440_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC476:
> > -	rs6000_cost = &ppc476_cost;
> > -	break;
> > +    case PROCESSOR_PPC476:
> > +      rs6000_cost = &ppc476_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC601:
> > -	rs6000_cost = &ppc601_cost;
> > -	break;
> > +    case PROCESSOR_PPC601:
> > +      rs6000_cost = &ppc601_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC603:
> > -	rs6000_cost = &ppc603_cost;
> > -	break;
> > +    case PROCESSOR_PPC603:
> > +      rs6000_cost = &ppc603_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC604:
> > -	rs6000_cost = &ppc604_cost;
> > -	break;
> > +    case PROCESSOR_PPC604:
> > +      rs6000_cost = &ppc604_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC604e:
> > -	rs6000_cost = &ppc604e_cost;
> > -	break;
> > +    case PROCESSOR_PPC604e:
> > +      rs6000_cost = &ppc604e_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC620:
> > -	rs6000_cost = &ppc620_cost;
> > -	break;
> > +    case PROCESSOR_PPC620:
> > +      rs6000_cost = &ppc620_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC630:
> > -	rs6000_cost = &ppc630_cost;
> > -	break;
> > +    case PROCESSOR_PPC630:
> > +      rs6000_cost = &ppc630_cost;
> > +      break;
> >  
> > -      case PROCESSOR_CELL:
> > -	rs6000_cost = &ppccell_cost;
> > -	break;
> > +    case PROCESSOR_CELL:
> > +      rs6000_cost = &ppccell_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC750:
> > -      case PROCESSOR_PPC7400:
> > -	rs6000_cost = &ppc750_cost;
> > -	break;
> > +    case PROCESSOR_PPC750:
> > +    case PROCESSOR_PPC7400:
> > +      rs6000_cost = &ppc750_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC7450:
> > -	rs6000_cost = &ppc7450_cost;
> > -	break;
> > +    case PROCESSOR_PPC7450:
> > +      rs6000_cost = &ppc7450_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPC8540:
> > -      case PROCESSOR_PPC8548:
> > -	rs6000_cost = &ppc8540_cost;
> > -	break;
> > +    case PROCESSOR_PPC8540:
> > +    case PROCESSOR_PPC8548:
> > +      rs6000_cost = &ppc8540_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCE300C2:
> > -      case PROCESSOR_PPCE300C3:
> > -	rs6000_cost = &ppce300c2c3_cost;
> > -	break;
> > +    case PROCESSOR_PPCE300C2:
> > +    case PROCESSOR_PPCE300C3:
> > +      rs6000_cost = &ppce300c2c3_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCE500MC:
> > -	rs6000_cost = &ppce500mc_cost;
> > -	break;
> > +    case PROCESSOR_PPCE500MC:
> > +      rs6000_cost = &ppce500mc_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCE500MC64:
> > -	rs6000_cost = &ppce500mc64_cost;
> > -	break;
> > +    case PROCESSOR_PPCE500MC64:
> > +      rs6000_cost = &ppce500mc64_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCE5500:
> > -	rs6000_cost = &ppce5500_cost;
> > -	break;
> > +    case PROCESSOR_PPCE5500:
> > +      rs6000_cost = &ppce5500_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCE6500:
> > -	rs6000_cost = &ppce6500_cost;
> > -	break;
> > +    case PROCESSOR_PPCE6500:
> > +      rs6000_cost = &ppce6500_cost;
> > +      break;
> >  
> > -      case PROCESSOR_TITAN:
> > -	rs6000_cost = &titan_cost;
> > -	break;
> > +    case PROCESSOR_TITAN:
> > +      rs6000_cost = &titan_cost;
> > +      break;
> >  
> > -      case PROCESSOR_POWER4:
> > -      case PROCESSOR_POWER5:
> > -	rs6000_cost = &power4_cost;
> > -	break;
> > +    case PROCESSOR_POWER4:
> > +    case PROCESSOR_POWER5:
> > +      rs6000_cost = &power4_cost;
> > +      break;
> >  
> > -      case PROCESSOR_POWER6:
> > -	rs6000_cost = &power6_cost;
> > -	break;
> > +    case PROCESSOR_POWER6:
> > +      rs6000_cost = &power6_cost;
> > +      break;
> >  
> > -      case PROCESSOR_POWER7:
> > -	rs6000_cost = &power7_cost;
> > -	break;
> > +    case PROCESSOR_POWER7:
> > +      rs6000_cost = &power7_cost;
> > +      break;
> >  
> > -      case PROCESSOR_POWER8:
> > -	rs6000_cost = &power8_cost;
> > -	break;
> > +    case PROCESSOR_POWER8:
> > +      rs6000_cost = &power8_cost;
> > +      break;
> >  
> > -      case PROCESSOR_POWER9:
> > -      case PROCESSOR_POWER10:
> > -	rs6000_cost = &power9_cost;
> > -	break;
> > +    case PROCESSOR_POWER9:
> > +    case PROCESSOR_POWER10:
> > +      rs6000_cost = &power9_cost;
> > +      break;
> >  
> > -      case PROCESSOR_PPCA2:
> > -	rs6000_cost = &ppca2_cost;
> > -	break;
> > +    case PROCESSOR_PPCA2:
> > +      rs6000_cost = &ppca2_cost;
> > +      break;
> >  
> > -      default:
> > -	gcc_unreachable ();
> > -      }
> > +    default:
> > +      gcc_unreachable ();
> > +    }
> >  
> >    if (global_init_p)
> >      {
> > @@ -21438,15 +21454,17 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  
> >      case PLUS:
> >      case MINUS:
> > -      if (FLOAT_MODE_P (mode))
> > +      if (speed && FLOAT_MODE_P (mode))
> >  	*total = rs6000_cost->fp;
> >        else
> >  	*total = COSTS_N_INSNS (1);
> >        return false;
> >  
> >      case MULT:
> > -      if (CONST_INT_P (XEXP (x, 1))
> > -	  && satisfies_constraint_I (XEXP (x, 1)))
> > +      if (!speed)
> > +	*total = COSTS_N_INSNS (1);
> > +      else if (CONST_INT_P (XEXP (x, 1))
> > +	       && satisfies_constraint_I (XEXP (x, 1)))
> >  	{
> >  	  if (INTVAL (XEXP (x, 1)) >= -256
> >  	      && INTVAL (XEXP (x, 1)) <= 255)
> > @@ -21465,7 +21483,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >        return false;
> >  
> >      case FMA:
> > -      if (mode == SFmode)
> > +      if (!speed)
> > +	*total = COSTS_N_INSNS (1);
> > +      else if (mode == SFmode)
> >  	*total = rs6000_cost->fp;
> >        else
> >  	*total = rs6000_cost->dmul;
> > @@ -21475,8 +21495,10 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >      case MOD:
> >        if (FLOAT_MODE_P (mode))
> >  	{
> > -	  *total = mode == DFmode ? rs6000_cost->ddiv
> > -				  : rs6000_cost->sdiv;
> > +	  if (!speed)
> > +	    *total = COSTS_N_INSNS (1);
> > +	  else
> > +	    *total = mode == DFmode ? rs6000_cost->ddiv : rs6000_cost->sdiv;
> >  	  return false;
> >  	}
> >        /* FALLTHRU */
> > @@ -21495,7 +21517,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >  	}
> >        else
> >  	{
> > -	  if (GET_MODE (XEXP (x, 1)) == DImode)
> > +	  if (!speed)
> > +	    *total = COSTS_N_INSNS (1);
> > +	  else if (GET_MODE (XEXP (x, 1)) == DImode)
> >  	    *total = rs6000_cost->divdi;
> >  	  else
> >  	    *total = rs6000_cost->divsi;
> > @@ -21587,7 +21611,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >        if (outer_code == TRUNCATE
> >  	  && GET_CODE (XEXP (x, 0)) == MULT)
> >  	{
> > -	  if (mode == DImode)
> > +	  if (!speed)
> > +	    *total = COSTS_N_INSNS (1);
> > +	  else if (mode == DImode)
> >  	    *total = rs6000_cost->muldi;
> >  	  else
> >  	    *total = rs6000_cost->mulsi;
> > @@ -21622,11 +21648,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
> >      case FIX:
> >      case UNSIGNED_FIX:
> >      case FLOAT_TRUNCATE:
> > -      *total = rs6000_cost->fp;
> > +      if (!speed)
> > +	*total = COSTS_N_INSNS (1);
> > +      else
> > +	*total = rs6000_cost->fp;
> >        return false;
> >  
> >      case FLOAT_EXTEND:
> > -      if (mode == DFmode)
> > +      if (!speed)
> > +	*total = COSTS_N_INSNS (1);
> > +      else if (mode == DFmode)
> >  	*total = rs6000_cost->sfdf_convert;
> >        else
> >  	*total = rs6000_cost->fp;
> > @@ -21773,6 +21804,10 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
> >        n = length / 4;
> >      }
> >  
> > +  const struct processor_costs *proc_cost = rs6000_cost;
> > +  if (optimize_size)
> > +    proc_cost = TARGET_POWERPC64 ? &size64_cost : &size32_cost;
> > +
> >    enum attr_type type = get_attr_type (insn);
> >  
> >    switch (type)
> > @@ -21787,16 +21822,16 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
> >        switch (get_attr_size (insn))
> >  	{
> >  	case SIZE_8:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const9;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi_const9;
> >  	  break;
> >  	case SIZE_16:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi_const;
> >  	  break;
> >  	case SIZE_32:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->mulsi;
> >  	  break;
> >  	case SIZE_64:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->muldi;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->muldi;
> >  	  break;
> >  	default:
> >  	  gcc_unreachable ();
> > @@ -21806,10 +21841,10 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
> >        switch (get_attr_size (insn))
> >  	{
> >  	case SIZE_32:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->divsi;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->divsi;
> >  	  break;
> >  	case SIZE_64:
> > -	  cost = COSTS_N_INSNS (n - 1) + rs6000_cost->divdi;
> > +	  cost = COSTS_N_INSNS (n - 1) + proc_cost->divdi;
> >  	  break;
> >  	default:
> >  	  gcc_unreachable ();
> > @@ -21817,16 +21852,16 @@ rs6000_insn_cost (rtx_insn *insn, bool speed)
> >        break;
> >  
> >      case TYPE_FP:
> > -      cost = n * rs6000_cost->fp;
> > +      cost = n * proc_cost->fp;
> >        break;
> >      case TYPE_DMUL:
> > -      cost = n * rs6000_cost->dmul;
> > +      cost = n * proc_cost->dmul;
> >        break;
> >      case TYPE_SDIV:
> > -      cost = n * rs6000_cost->sdiv;
> > +      cost = n * proc_cost->sdiv;
> >        break;
> >      case TYPE_DDIV:
> > -      cost = n * rs6000_cost->ddiv;
> > +      cost = n * proc_cost->ddiv;
> >        break;
> >  
> >      case TYPE_SYNC:
> > @@ -22388,7 +22423,7 @@ rs6000_emit_parity (rtx dst, rtx src)
> >    if (mode == SImode)
> >      {
> >        /* Is mult+shift >= shift+xor+shift+xor?  */
> > -      if (rs6000_cost->mulsi_const >= COSTS_N_INSNS (3))
> > +      if (!optimize_size && rs6000_cost->mulsi_const >= COSTS_N_INSNS (3))
> >  	{
> >  	  rtx tmp1, tmp2, tmp3, tmp4;
> >  
> > @@ -22411,7 +22446,7 @@ rs6000_emit_parity (rtx dst, rtx src)
> >    else
> >      {
> >        /* Is mult+shift >= shift+xor+shift+xor+shift+xor?  */
> > -      if (rs6000_cost->muldi >= COSTS_N_INSNS (5))
> > +      if (!optimize_size && rs6000_cost->muldi >= COSTS_N_INSNS (5))
> >  	{
> >  	  rtx tmp1, tmp2, tmp3, tmp4, tmp5, tmp6;
> >  
> > diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> > index bbd8060e143..9daf55adc02 100644
> > --- a/gcc/config/rs6000/rs6000.h
> > +++ b/gcc/config/rs6000/rs6000.h
> > @@ -1869,29 +1869,6 @@ extern scalar_int_mode rs6000_pmode;
> >  #define REVERSE_CONDITION(CODE, MODE) rs6000_reverse_condition (MODE, CODE)
> >  
> >  \f
> > -/* Target cpu costs.  */
> > -
> > -struct processor_costs {
> > -  const int mulsi;	  /* cost of SImode multiplication.  */
> > -  const int mulsi_const;  /* cost of SImode multiplication by constant.  */
> > -  const int mulsi_const9; /* cost of SImode mult by short constant.  */
> > -  const int muldi;	  /* cost of DImode multiplication.  */
> > -  const int divsi;	  /* cost of SImode division.  */
> > -  const int divdi;	  /* cost of DImode division.  */
> > -  const int fp;		  /* cost of simple SFmode and DFmode insns.  */
> > -  const int dmul;	  /* cost of DFmode multiplication (and fmadd).  */
> > -  const int sdiv;	  /* cost of SFmode division (fdivs).  */
> > -  const int ddiv;	  /* cost of DFmode division (fdiv).  */
> > -  const int cache_line_size;    /* cache line size in bytes. */
> > -  const int l1_cache_size;	/* size of l1 cache, in kilobytes.  */
> > -  const int l2_cache_size;	/* size of l2 cache, in kilobytes.  */
> > -  const int simultaneous_prefetches; /* number of parallel prefetch
> > -					operations.  */
> > -  const int sfdf_convert;	/* cost of SF->DF conversion.  */
> > -};
> > -
> > -extern const struct processor_costs *rs6000_cost;
> > -\f
> >  /* Control the assembler format that we output.  */
> >  
> >  /* A C string constant describing how to begin a comment in the target

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND

[Segher Boessenkool]

Hi!

On Thu, Oct 08, 2020 at 09:27:55AM +1030, Alan Modra wrote:
> 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy AND code.
> 	Don't avoid recursion on const_int shift count.
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index e870ba0039a..bc5e51aa5ce 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -21253,6 +21253,7 @@ static bool
>  rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>  		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
>  {
> +  rtx right;

Please declare things where you first use them.

>    int code = GET_CODE (x);
>  
>    switch (code)
> @@ -21430,7 +21431,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>        return false;
>  
>      case AND:
> -      if (CONST_INT_P (XEXP (x, 1)))
> +      *total = COSTS_N_INSNS (1);
> +      right = XEXP (x, 1);
> +      if (CONST_INT_P (right))
>  	{
>  	  rtx left = XEXP (x, 0);
>  	  rtx_code left_code = GET_CODE (left);
> @@ -21439,17 +21442,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>  	  if ((left_code == ROTATE
>  	       || left_code == ASHIFT
>  	       || left_code == LSHIFTRT)
> -	      && rs6000_is_valid_shift_mask (XEXP (x, 1), left, mode))
> +	      && rs6000_is_valid_shift_mask (right, left, mode))

You could have left all this intact, it is just distraction from the
rest of the patch (nothing changed here).  Except you set *total.

>  	    {
> -	      *total = rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
> -	      if (!CONST_INT_P (XEXP (left, 1)))
> -		*total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
> -	      *total += COSTS_N_INSNS (1);
> +	      *total += rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
> +	      *total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
>  	      return true;
>  	    }
>  	}
> -
> -      *total = COSTS_N_INSNS (1);
>        return false;

I still do not see what this improves, I only see possible obvious
regressions :-(


Segher

Re: [PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return

[Segher Boessenkool]

On Thu, Oct 08, 2020 at 09:27:56AM +1030, Alan Modra wrote:
> Most cases use "return false" rather than breaking out of the switch.
> Do so in all cases.

>      default:
> -      break;
> +      return false;
>      }
> -
> -  return false;
>  }

Please don't do this part.  The rest is okay.  Thanks!


Segher

Re: [PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR

[Segher Boessenkool]

Hi!

On Thu, Oct 08, 2020 at 09:27:57AM +1030, Alan Modra wrote:
> 	* config/rs6000/rs6000.c (rotate_insert_cost): New function.
> 	(rs6000_rtx_costs): Cost IOR.
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 383d2901c9f..15a806fe307 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
>  	 && get_attr_cannot_copy (insn);
>  }
>  
> +/* Handle rtx_costs for scalar integer rotate and insert insns.  */

You need to document here what the return value means, and what the
preconditions for "left" (and "right") are.

> +static bool
> +rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
> +		    int *total)
> +{
> +  if (GET_CODE (right) == AND

... because you never check the CODE of "left".

> +      && CONST_INT_P (XEXP (right, 1))
> +      && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)

HOST_WIDE_INT is always exactly 64 bits now, so you could do "== -1".

> +    {
> +      rtx leftop = XEXP (left, 0);
> +      rtx rightop = XEXP (right, 0);
> +
> +      /* rotlsi3_insert_5.  */
> +      if (REG_P (leftop)
> +	  && REG_P (rightop)
> +	  && mode == SImode
> +	  && UINTVAL (XEXP (left, 1)) != 0
> +	  && UINTVAL (XEXP (right, 1)) != 0
> +	  && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
> +	return true;

Empty line after return please.

> +      /* rotldi3_insert_6.  */
> +      if (REG_P (leftop)
> +	  && REG_P (rightop)
> +	  && mode == DImode
> +	  && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
> +	return true;
> +      /* rotldi3_insert_7.  */
> +      if (REG_P (leftop)
> +	  && REG_P (rightop)
> +	  && mode == DImode
> +	  && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
> +	return true;

Those could just use rs6000_is_valid_mask as well?

> +      rtx mask = 0;
> +      rtx shift = leftop;
> +      rtx_code shift_code = GET_CODE (shift);
> +      /* rotl<mode>3_insert.  */
> +      if (shift_code == ROTATE
> +	  || shift_code == ASHIFT
> +	  || shift_code == LSHIFTRT)
> +	mask = right;
> +      else
> +	{
> +	  shift = rightop;
> +	  shift_code = GET_CODE (shift);
> +	  /* rotl<mode>3_insert_2.  */
> +	  if (shift_code == ROTATE
> +	      || shift_code == ASHIFT
> +	      || shift_code == LSHIFTRT)
> +	    mask = left;
> +	}
> +      if (mask
> +	  && CONST_INT_P (XEXP (shift, 1))
> +	  && rs6000_is_valid_insert_mask (XEXP (mask, 1), shift, mode))
> +	{
> +	  *total += rtx_cost (XEXP (shift, 0), mode, shift_code, 0, speed);
> +	  *total += rtx_cost (XEXP (mask, 0), mode, AND, 0, speed);
> +	  return true;
> +	}
> +    }
> +  /* rotl<mode>3_insert_3.  */
> +  if (GET_CODE (right) == ASHIFT
> +      && CONST_INT_P (XEXP (right, 1))
> +      && (INTVAL (XEXP (right, 1))
> +	  == exact_log2 (UINTVAL (XEXP (left, 1)) + 1)))
> +    {
> +      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
> +      *total += rtx_cost (XEXP (right, 0), mode, ASHIFT, 0, speed);
> +      return true;
> +    }
> +  /* rotl<mode>3_insert_4.  */
> +  if (GET_CODE (right) == LSHIFTRT
> +      && CONST_INT_P (XEXP (right, 1))
> +      && mode == SImode
> +      && (INTVAL (XEXP (right, 1))
> +	  + exact_log2 (-UINTVAL (XEXP (left, 1)))) == 32)
> +    {
> +      *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
> +      *total += rtx_cost (XEXP (right, 0), mode, LSHIFTRT, 0, speed);
> +      return true;
> +    }
> +  return false;
> +}
> +
>  /* Compute a (partial) cost for rtx X.  Return true if the complete
>     cost has been computed, and false if subexpressions should be
>     scanned.  In either case, *TOTAL contains the cost result.
> @@ -21253,7 +21338,7 @@ static bool
>  rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>  		  int opno ATTRIBUTE_UNUSED, int *total, bool speed)
>  {
> -  rtx right;
> +  rtx left, right;
>    int code = GET_CODE (x);
>  
>    switch (code)
> @@ -21435,7 +21520,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>        right = XEXP (x, 1);
>        if (CONST_INT_P (right))
>  	{
> -	  rtx left = XEXP (x, 0);
> +	  left = XEXP (x, 0);
>  	  rtx_code left_code = GET_CODE (left);
>  
>  	  /* rotate-and-mask: 1 insn.  */
> @@ -21452,9 +21537,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
>        return false;
>  
>      case IOR:
> -      /* FIXME */
>        *total = COSTS_N_INSNS (1);
> -      return true;
> +      left = XEXP (x, 0);
> +      if (GET_CODE (left) == AND
> +	  && CONST_INT_P (XEXP (left, 1)))
> +	{
> +	  right = XEXP (x, 1);
> +	  if (rotate_insert_cost (left, right, mode, speed, total))
> +	    return true;
> +	}
> +      return false;
>  
>      case CLZ:
>      case XOR:

Please wait this until stage 1.  Sorry.


Segher

Re: [PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND

[Alan Modra]

On Mon, Jan 25, 2021 at 04:37:21PM -0600, Segher Boessenkool wrote:
> I still do not see what this improves, I only see possible obvious
> regressions :-(

You asked me to break the patch series into small pieces, for ease of
review and to separate tidies from functional changes.  Well OK, fair
enough.  This is one of the tidies.  The idea being to make
rs6000_rtx_costs a little more self-consistent, to not have someone
look at this code in the future and wonder why AND was treated
differently to other operations.

The only part of this patch that I can imagine you see as a possible
regression is the "Don't avoid recursion on const_int shift count"
part.  That is there only because you wanted it that way in new code.
I think you said something about premature optimisation when I made
the new code special case const_int and reg to stop recursion, like
AND.  So for consistency I made the change in old code too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR

[Alan Modra]

On Mon, Jan 25, 2021 at 04:51:43PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Oct 08, 2020 at 09:27:57AM +1030, Alan Modra wrote:
> > 	* config/rs6000/rs6000.c (rotate_insert_cost): New function.
> > 	(rs6000_rtx_costs): Cost IOR.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 383d2901c9f..15a806fe307 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
> >  	 && get_attr_cannot_copy (insn);
> >  }
> >  
> > +/* Handle rtx_costs for scalar integer rotate and insert insns.  */
> 
> You need to document here what the return value means, and what the
> preconditions for "left" (and "right") are.

Done, and I moved the preconditions on "left" into the new function.

> > +static bool
> > +rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
> > +		    int *total)
> > +{
> > +  if (GET_CODE (right) == AND
> 
> ... because you never check the CODE of "left".
> 
> > +      && CONST_INT_P (XEXP (right, 1))
> > +      && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)
> 
> HOST_WIDE_INT is always exactly 64 bits now, so you could do "== -1".

Yes, but this is exactly the way the expression occurs in rotl*_insert*
instruction patterns.  I think it's better to keep them the same.

> > +    {
> > +      rtx leftop = XEXP (left, 0);
> > +      rtx rightop = XEXP (right, 0);
> > +
> > +      /* rotlsi3_insert_5.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == SImode
> > +	  && UINTVAL (XEXP (left, 1)) != 0
> > +	  && UINTVAL (XEXP (right, 1)) != 0
> > +	  && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
> > +	return true;
> 
> Empty line after return please.

Done, here and elsewhere.

> > +      /* rotldi3_insert_6.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == DImode
> > +	  && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
> > +	return true;
> > +      /* rotldi3_insert_7.  */
> > +      if (REG_P (leftop)
> > +	  && REG_P (rightop)
> > +	  && mode == DImode
> > +	  && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
> > +	return true;
> 
> Those could just use rs6000_is_valid_mask as well?

This is taken straight from rotldi3_insert_7, so it really ought to
stay that way.

> 
> Please wait this until stage 1.  Sorry.

OK, I'll leave all the rs6000_rtx_costs changes until then.

-- 
Alan Modra
Australia Development Lab, IBM