diff mbox

[1/9] gensupport: Fix define_subst operand renumbering.

Message ID 1452789254-12603-2-git-send-email-krebbel@linux.vnet.ibm.com
State New
Headers show

Commit Message

Andreas Krebbel Jan. 14, 2016, 4:33 p.m. UTC 
When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used to record
the operand numbers already in use.  Currently this array is used to
assign new numbers *before* all the RTXes in the vector have been
processed.

I did run into problems with this for insns where a match_dup occurred
in a later (use ...) operand referring to an earlier operand
(e.g. s390.md "setmem_long").

The patch splits the loop doing the processing into two in order to
have all the operand numbers recorded already when assigning new
numbers.

Bootstrapped and regtested on s390, s390x, and x86_64.

Ok for mainline?

Bye,

-Andreas-

gcc/ChangeLog:

2016-01-14  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	* gensupport.c (process_substs_on_one_elem): Split loop to
	complete mark_operands_used_in_match_dup on all expressions in the
	vector first.
	(adjust_operands_numbers): Inline into process_substs_on_one_elem
	and remove function.
---
 gcc/gensupport.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

Comments

Bernd Schmidt Jan. 18, 2016, 2:46 p.m. UTC | #1 
On 01/14/2016 05:33 PM, Andreas Krebbel wrote:
> When processing substitutions the operands are renumbered.  To find a
> free operand number the array used_operands_numbers is used to record
> the operand numbers already in use.  Currently this array is used to
> assign new numbers *before* all the RTXes in the vector have been
> processed.

> 	* gensupport.c (process_substs_on_one_elem): Split loop to
> 	complete mark_operands_used_in_match_dup on all expressions in the
> 	vector first.
> 	(adjust_operands_numbers): Inline into process_substs_on_one_elem
> 	and remove function.

Mostly ok, I think. As an aside, all the define_subst stuff in 
gensupport looks rather suspiciously clunky and the comments are in 
broken English. We should fix this stuff at some point.

> @@ -1976,6 +1986,14 @@ find_first_unused_number_of_operand ()
>      It visits all expressions in PATTERN and assigns not-occupied
>      operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
>      PATTERN.  */
> +/* If output pattern of define_subst contains MATCH_DUP, then this
> +   expression would be replaced with the pattern, matched with
> +   MATCH_OPERAND from input pattern.  This pattern could contain any
> +   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
> +   that a MATCH_OPERAND from output_pattern (if any) would have the
> +   same number, as MATCH_OPERAND from copied pattern.  To avoid such
> +   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
> +   laying in the output pattern outside of MATCH_DUPs.  */
>   static void
>   renumerate_operands_in_pattern (rtx pattern)
>   {

If you want to keep this comment, you might want to move it inside the 
function (or into the caller). Ok with or without any such change - this 
looks a bit weird but I don't know what's best.


Bernd
diff mbox

Patch

diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 596b885..8ace425 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -126,7 +126,10 @@  static const char * duplicate_each_alternative (const char * str, int n_dup);
 
 typedef const char * (*constraints_handler_t) (const char *, int);
 static rtx alter_constraints (rtx, int, constraints_handler_t);
-static rtx adjust_operands_numbers (rtx);
+
+static void mark_operands_used_in_match_dup (rtx);
+static void renumerate_operands_in_pattern (rtx);
+
 static rtx replace_duplicating_operands_in_pattern (rtx);
 
 /* Make a version of gen_rtx_CONST_INT so that GEN_INT can be used in
@@ -1843,7 +1846,15 @@  process_substs_on_one_elem (struct queue_elem *elem,
 	  subst_pattern = alter_constraints (subst_pattern, alternatives,
 					     duplicate_each_alternative);
 
-	  subst_pattern = adjust_operands_numbers (subst_pattern);
+	  mark_operands_used_in_match_dup (subst_pattern);
+	  RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
+	}
+
+      for (j = 0; j < XVECLEN (subst_elem->data, 3); j++)
+	{
+	  subst_pattern = RTVEC_ELT (subst_pattern_vec, j);
+
+	  renumerate_operands_in_pattern (subst_pattern);
 
 	  /* Substitute match_dup and match_op_dup in the new pattern and
 	     duplicate constraints.  */
@@ -1856,7 +1867,6 @@  process_substs_on_one_elem (struct queue_elem *elem,
 	  if (GET_CODE (elem->data) == DEFINE_EXPAND)
 	    remove_constraints (subst_pattern);
 
-	  RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
 	}
       XVEC (elem->data, 1) = subst_pattern_vec;
 
@@ -1976,6 +1986,14 @@  find_first_unused_number_of_operand ()
    It visits all expressions in PATTERN and assigns not-occupied
    operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
    PATTERN.  */
+/* If output pattern of define_subst contains MATCH_DUP, then this
+   expression would be replaced with the pattern, matched with
+   MATCH_OPERAND from input pattern.  This pattern could contain any
+   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
+   that a MATCH_OPERAND from output_pattern (if any) would have the
+   same number, as MATCH_OPERAND from copied pattern.  To avoid such
+   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
+   laying in the output pattern outside of MATCH_DUPs.  */
 static void
 renumerate_operands_in_pattern (rtx pattern)
 {
@@ -2010,23 +2028,6 @@  renumerate_operands_in_pattern (rtx pattern)
     }
 }
 
-/* If output pattern of define_subst contains MATCH_DUP, then this
-   expression would be replaced with the pattern, matched with
-   MATCH_OPERAND from input pattern.  This pattern could contain any
-   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
-   that a MATCH_OPERAND from output_pattern (if any) would have the
-   same number, as MATCH_OPERAND from copied pattern.  To avoid such
-   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
-   laying in the output pattern outside of MATCH_DUPs.  */
-static rtx
-adjust_operands_numbers (rtx pattern)
-{
-  mark_operands_used_in_match_dup (pattern);
-
-  renumerate_operands_in_pattern (pattern);
-
-  return pattern;
-}
 
 /* Generate RTL expression
    (match_dup OPNO)