diff mbox series

Try harder to preserve operand ties in maybe_legitimize_operands

Message ID 87in7elvfn.fsf@linaro.org
State Accepted
Commit b883fc9b54d360b5f73bf226bb489e8612502298
Headers show
Series Try harder to preserve operand ties in maybe_legitimize_operands | expand

Commit Message

Richard Sandiford May 23, 2018, 6:47 a.m. UTC 
maybe_legitimize_operands normally goes through each operand in turn
and legitimises it in isolation.  For example, if two operands to
an instruction initially have constant value C, and the instruction
requires both operands to be registers, the function ends up forcing
C into a register twice and passing two different registers to the
instruction.

I think we should try a bit harder to preserve the rtx_equal_p
property, if it's easy to do.  Some targets can optimise that
case better than they would the general case of all operands
being different.  This is particularly true for SVE after the
upcoming changes to the IFN_COND_* routines.

This is hard to test on its own, but is covered by the upcoming
IFN_COND_* patches.

Tested on aarch64-linux-gnu (with and without SLP, and with and without
the follow-on patch that needs it), aarch64_be-elf and x86_64-linux-gnu.
OK to install?

Richard


2018-05-23  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* optabs.c (can_reuse_operands_p): New function.
	(maybe_legitimize_operands): Try to reuse the results for
	earlier operands.

Comments

Jeff Law May 24, 2018, 10:48 p.m. UTC | #1 
On 05/23/2018 12:47 AM, Richard Sandiford wrote:
> maybe_legitimize_operands normally goes through each operand in turn

> and legitimises it in isolation.  For example, if two operands to

> an instruction initially have constant value C, and the instruction

> requires both operands to be registers, the function ends up forcing

> C into a register twice and passing two different registers to the

> instruction.

> 

> I think we should try a bit harder to preserve the rtx_equal_p

> property, if it's easy to do.  Some targets can optimise that

> case better than they would the general case of all operands

> being different.  This is particularly true for SVE after the

> upcoming changes to the IFN_COND_* routines.

> 

> This is hard to test on its own, but is covered by the upcoming

> IFN_COND_* patches.

> 

> Tested on aarch64-linux-gnu (with and without SLP, and with and without

> the follow-on patch that needs it), aarch64_be-elf and x86_64-linux-gnu.

> OK to install?

> 

> Richard

> 

> 

> 2018-05-23  Richard Sandiford  <richard.sandiford@linaro.org>

> 

> gcc/

> 	* optabs.c (can_reuse_operands_p): New function.

> 	(maybe_legitimize_operands): Try to reuse the results for

> 	earlier operands.

OK.
jeff
diff mbox series

Patch

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2018-03-17 08:30:19.229122223 +0000
+++ gcc/optabs.c	2018-05-23 07:45:57.699793258 +0100
@@ -7207,6 +7207,44 @@  create_convert_operand_from_type (struct
 			       TYPE_UNSIGNED (type));
 }
 
+/* Return true if the requirements on operands OP1 and OP2 of instruction
+   ICODE are similar enough for the result of legitimizing OP1 to be
+   reusable for OP2.  OPNO1 and OPNO2 are the operand numbers associated
+   with OP1 and OP2 respectively.  */
+
+static inline bool
+can_reuse_operands_p (enum insn_code icode,
+		      unsigned int opno1, unsigned int opno2,
+		      const struct expand_operand *op1,
+		      const struct expand_operand *op2)
+{
+  /* Check requirements that are common to all types.  */
+  if (op1->type != op2->type
+      || op1->mode != op2->mode
+      || (insn_data[(int) icode].operand[opno1].mode
+	  != insn_data[(int) icode].operand[opno2].mode))
+    return false;
+
+  /* Check the requirements for specific types.  */
+  switch (op1->type)
+    {
+    case EXPAND_OUTPUT:
+      /* Outputs must remain distinct.  */
+      return false;
+
+    case EXPAND_FIXED:
+    case EXPAND_INPUT:
+    case EXPAND_ADDRESS:
+    case EXPAND_INTEGER:
+      return true;
+
+    case EXPAND_CONVERT_TO:
+    case EXPAND_CONVERT_FROM:
+      return op1->unsigned_p == op2->unsigned_p;
+    }
+  gcc_unreachable ();
+}
+
 /* Try to make operands [OPS, OPS + NOPS) match operands [OPNO, OPNO + NOPS)
    of instruction ICODE.  Return true on success, leaving the new operand
    values in the OPS themselves.  Emit no code on failure.  */
@@ -7215,16 +7253,35 @@  create_convert_operand_from_type (struct
 maybe_legitimize_operands (enum insn_code icode, unsigned int opno,
 			   unsigned int nops, struct expand_operand *ops)
 {
-  rtx_insn *last;
-  unsigned int i;
+  rtx_insn *last = get_last_insn ();
+  rtx *orig_values = XALLOCAVEC (rtx, nops);
+  for (unsigned int i = 0; i < nops; i++)
+    {
+      orig_values[i] = ops[i].value;
+
+      /* First try reusing the result of an earlier legitimization.
+	 This avoids duplicate rtl and ensures that tied operands
+	 remain tied.
+
+	 This search is linear, but NOPS is bounded at compile time
+	 to a small number (current a single digit).  */
+      unsigned int j = 0;
+      for (; j < i; ++j)
+	if (can_reuse_operands_p (icode, opno + j, opno + i, &ops[j], &ops[i])
+	    && rtx_equal_p (orig_values[j], orig_values[i])
+	    && insn_operand_matches (icode, opno + i, ops[j].value))
+	  {
+	    ops[i].value = copy_rtx (ops[j].value);
+	    break;
+	  }
 
-  last = get_last_insn ();
-  for (i = 0; i < nops; i++)
-    if (!maybe_legitimize_operand (icode, opno + i, &ops[i]))
-      {
-	delete_insns_since (last);
-	return false;
-      }
+      /* Otherwise try legitimizing the operand on its own.  */
+      if (j == i && !maybe_legitimize_operand (icode, opno + i, &ops[i]))
+	{
+	  delete_insns_since (last);
+	  return false;
+	}
+    }
   return true;
 }