public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* commutative asm operands
@ 2002-07-31  9:45 Alan Modra
  2002-07-31 22:59 ` Richard Henderson
  2002-08-03 14:19 ` Roman Zippel
  0 siblings, 2 replies; 10+ messages in thread
From: Alan Modra @ 2002-07-31  9:45 UTC (permalink / raw)
  To: gcc

I was forwarded a testcase today containing some horrible asm

asm ("addc %0, %2, %3\n\t"
     "adde %1, %4, %5"
     : "=r" (lo), "=r" (hi)
     : "%r" (__lo), "0" (lo), "%r" (__hi), "1" (hi))

Note the use of "%" with "0"/"1" in the next operand.  Is this legal?
If so, recog.c:constrain_operands needs to be taught to handle this
case.

Led to an ICE on powerpc64 gcc-3.1.1

unrecognizable insn:
(insn 1181 5394 1182 (parallel[ 
            (set (reg:SI 0 r0 [742])
                (asm_operands:SI ("addc %0, %2, %3
        adde %1, %4, %5") ("=r") 0[ 
                        (reg:SI 0 r0 [737])
                        (reg:SI 30 r30 [134])
                        (reg:SI 31 r31 [133])
                        (reg:SI 9 r9 [743])
                    ] 
                    [ 
                        (asm_input:SI ("%r"))
                        (asm_input:SI ("0"))
                        (asm_input:SI ("%r"))
                        (asm_input:SI ("1"))
                    ]  ("layer3.c") 1698))
            (set (reg:SI 9 r9 [743])
                (asm_operands:SI ("addc %0, %2, %3
        adde %1, %4, %5") ("=r") 1[ 
                        (reg:SI 0 r0 [737])
                        (reg:SI 30 r30 [134])
                        (reg:SI 31 r31 [133])
                        (reg:SI 9 r9 [743])
                    ] 
                    [ 
                        (asm_input:SI ("%r"))
                        (asm_input:SI ("0"))
                        (asm_input:SI ("%r"))
                        (asm_input:SI ("1"))
                    ]  ("layer3.c") 1698))
        ] ) -1 (insn_list 1179 (insn_list 1180 (insn_list 1168 (insn_list 1169 (nil)))))
    (nil))
Internal compiler error in reload_cse_simplify_operands, at reload1.c:8369

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-07-31  9:45 commutative asm operands Alan Modra
@ 2002-07-31 22:59 ` Richard Henderson
  2002-08-03 14:19 ` Roman Zippel
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2002-07-31 22:59 UTC (permalink / raw)
  To: gcc

On Wed, Jul 31, 2002 at 11:44:25PM +0930, Alan Modra wrote:
> I was forwarded a testcase today containing some horrible asm
> 
> asm ("addc %0, %2, %3\n\t"
>      "adde %1, %4, %5"
>      : "=r" (lo), "=r" (hi)
>      : "%r" (__lo), "0" (lo), "%r" (__hi), "1" (hi))
> 
> Note the use of "%" with "0"/"1" in the next operand.  Is this legal?

Dunno.  The x86 port does this the other way around -- puts 
the % with the matching constraint.  I.e.

  : "%0" (lo), "r"(__lo)


r~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-07-31  9:45 commutative asm operands Alan Modra
  2002-07-31 22:59 ` Richard Henderson
@ 2002-08-03 14:19 ` Roman Zippel
  2002-08-03 19:45   ` Alan Modra
  1 sibling, 1 reply; 10+ messages in thread
From: Roman Zippel @ 2002-08-03 14:19 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc

Hi,

Alan Modra wrote:
> I was forwarded a testcase today containing some horrible asm
> 
> asm ("addc %0, %2, %3\n\t"
>      "adde %1, %4, %5"
>      : "=r" (lo), "=r" (hi)
>      : "%r" (__lo), "0" (lo), "%r" (__hi), "1" (hi))

I have a patch to fix this problem, but I have to update and test it again.

bye, Roman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-03 14:19 ` Roman Zippel
@ 2002-08-03 19:45   ` Alan Modra
  2002-08-04  9:29     ` Roman Zippel
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2002-08-03 19:45 UTC (permalink / raw)
  To: Roman Zippel; +Cc: gcc, gcc-patches

On Sat, Aug 03, 2002 at 11:19:18PM +0200, Roman Zippel wrote:
> Hi,
> 
> Alan Modra wrote:
> > I was forwarded a testcase today containing some horrible asm
> > 
> > asm ("addc %0, %2, %3\n\t"
> >      "adde %1, %4, %5"
> >      : "=r" (lo), "=r" (hi)
> >      : "%r" (__lo), "0" (lo), "%r" (__hi), "1" (hi))
> 
> I have a patch to fix this problem, but I have to update and test it again.

Hey, so have I!  I'd better post it..

	* recog.c (constrain_operands): Handle commutative operands.

Bootstrapped, regtested powerpc-linux.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Index: recog.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.162
diff -u -p -r1.162 recog.c
--- recog.c	23 Jul 2002 20:50:59 -0000	1.162
+++ recog.c	3 Aug 2002 13:08:02 -0000
@@ -2332,10 +2332,12 @@ constrain_operands (strict)
      int strict;
 {
   const char *constraints[MAX_RECOG_OPERANDS];
+  const char *last_constraints[MAX_RECOG_OPERANDS];
   int matching_operands[MAX_RECOG_OPERANDS];
   int earlyclobber[MAX_RECOG_OPERANDS];
   int c;
-
+  int commutative;
+  bool swapped;
   struct funny_match funny_match[MAX_RECOG_OPERANDS];
   int funny_match_index;
 
@@ -2349,6 +2351,8 @@ constrain_operands (strict)
       matching_operands[c] = -1;
     }
 
+  swapped = false;
+  commutative = 0;
   do
     {
       int opno;
@@ -2390,7 +2394,12 @@ constrain_operands (strict)
 	  while (*p && (c = *p++) != ',')
 	    switch (c)
 	      {
-	      case '?':  case '!': case '*':  case '%':
+	      case '%':
+		if (! lose && ! swapped)
+		  commutative = opno + 1;
+		break;
+
+	      case '?':  case '!': case '*':
 	      case '=':  case '+':
 		break;
 
@@ -2605,11 +2614,12 @@ constrain_operands (strict)
 		}
 	      }
 
+	  last_constraints[opno] = constraints[opno];
 	  constraints[opno] = p;
 	  /* If this operand did not win somehow,
 	     this alternative loses.  */
-	  if (! win)
-	    lose = 1;
+	  if (! win && ! lose)
+	    lose = opno + 1;
 	}
       /* This alternative won; the operands are ok.
 	 Change whichever operands this alternative says to change.  */
@@ -2638,7 +2648,7 @@ constrain_operands (strict)
 						 recog_data.operand[eopno]))
 		      && ! safe_from_earlyclobber (recog_data.operand[opno],
 						   recog_data.operand[eopno]))
-		    lose = 1;
+		    lose = opno + 1;
 
 	  if (! lose)
 	    {
@@ -2652,7 +2662,28 @@ constrain_operands (strict)
 	    }
 	}
 
-      which_alternative++;
+      if (! swapped
+	  && commutative
+	  && (lose == commutative || lose == commutative + 1))
+	{
+	  for (opno = 0; opno < recog_data.n_operands; opno++)
+	    constraints[opno] = last_constraints[opno];
+	  constraints[commutative - 1] = last_constraints[commutative];
+	  constraints[commutative] = last_constraints[commutative - 1];
+	  swapped = true;
+	}
+      else
+	{
+	  if (swapped)
+	    {
+	      const char *tmp = constraints[commutative - 1];
+	      constraints[commutative - 1] = constraints[commutative];
+	      constraints[commutative] = tmp;
+	      swapped = false;
+	    }
+	  commutative = 0;
+	  which_alternative++;
+	}
     }
   while (which_alternative < recog_data.n_alternatives);
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-03 19:45   ` Alan Modra
@ 2002-08-04  9:29     ` Roman Zippel
  2002-08-04 14:46       ` Alan Modra
  0 siblings, 1 reply; 10+ messages in thread
From: Roman Zippel @ 2002-08-04  9:29 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

Hi,

Alan Modra wrote:

> Hey, so have I!  I'd better post it..
> 
>         * recog.c (constrain_operands): Handle commutative operands.

What exactly does this patch fix?
What is the assembly output for this test case (register names might
need a change):

int f()
{
        register int a asm("%ebx");
        register int b asm("%ecx");
        register int c asm("%edx");
        register int d asm("%edi");

        asm volatile ("addc %0, %2, %3\n\tadde %1, %4, %5"
                : "=r" (a), "=r" (b)
                : "%0" (a), "r" (c), "%1" (b), "r" (d));
        asm volatile ("addc %0, %2, %3\n\tadde %1, %4, %5"
                : "=r" (a), "=r" (b)
                : "%0" (c), "r" (a), "%1" (d), "r" (b));
}

I attached my patch, it still applies, but it's untested for a while.
That patch really tries to find the best combination of commutative
operands.

bye, Roman

[-- Attachment #2: commutative-fix.diff --]
[-- Type: text/plain, Size: 11286 bytes --]

Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload.c,v
retrieving revision 1.180
diff -u -p -r1.180 reload.c
--- reload.c	2002/03/10 23:51:08	1.180
+++ reload.c	2002/04/03 23:21:30
@@ -2449,6 +2449,7 @@ find_reloads (insn, replace, ind_levels,
   char this_alternative_earlyclobber[MAX_RECOG_OPERANDS];
   int this_alternative_matches[MAX_RECOG_OPERANDS];
   int swapped;
+  int n_permutation;
   int goal_alternative[MAX_RECOG_OPERANDS];
   int this_alternative_number;
   int goal_alternative_number = 0;
@@ -2461,7 +2462,8 @@ find_reloads (insn, replace, ind_levels,
   char goal_alternative_earlyclobber[MAX_RECOG_OPERANDS];
   int goal_alternative_swapped;
   int best;
-  int commutative;
+  int commutative[MAX_RECOG_OPERANDS];
+  int commutative_op[MAX_RECOG_OPERANDS/2];
   char operands_match[MAX_RECOG_OPERANDS][MAX_RECOG_OPERANDS];
   rtx substed_operand[MAX_RECOG_OPERANDS];
   rtx body = PATTERN (insn);
@@ -2527,14 +2529,14 @@ find_reloads (insn, replace, ind_levels,
 	  noperands * sizeof (enum machine_mode));
   memcpy (constraints, recog_data.constraints, noperands * sizeof (char *));
 
-  commutative = -1;
-
   /* If we will need to know, later, whether some pair of operands
      are the same, we must compare them now and save the result.
      Reloading the base and index registers will clobber them
      and afterward they will fail to match.  */
-
   for (i = 0; i < noperands; i++)
+    commutative[i] = i;
+
+  for (i = 0, j = 0; i < noperands; i++)
     {
       char *p;
       int c;
@@ -2555,11 +2557,14 @@ find_reloads (insn, replace, ind_levels,
 	    modified[i] = RELOAD_READ_WRITE;
 	  else if (c == '%')
 	    {
-	      /* The last operand should not be marked commutative.  */
-	      if (i == noperands - 1)
+	      /* The last operand and two consecutive operands should not be
+	          marked commutative.  */
+	      if (i == noperands - 1 || commutative[i] != i)
 		abort ();
 
-	      commutative = i;
+	      commutative_op[j++] = i;
+	      commutative[i] = i + 1;
+	      commutative[i + 1] = i;
 	    }
 	  else if (ISDIGIT (c))
 	    {
@@ -2573,32 +2578,31 @@ find_reloads (insn, replace, ind_levels,
 	      if (c == i)
 		abort ();
 
+	      if (commutative[i] != i)
+		operands_match[c][commutative[i]]
+		  = operands_match_p (recog_data.operand[c],
+				      recog_data.operand[commutative[i]]);
+
 	      /* If C can be commuted with C+1, and C might need to match I,
 		 then C+1 might also need to match I.  */
-	      if (commutative >= 0)
-		{
-		  if (c == commutative || c == commutative + 1)
-		    {
-		      int other = c + (c == commutative ? 1 : -1);
-		      operands_match[other][i]
-			= operands_match_p (recog_data.operand[other],
-					    recog_data.operand[i]);
-		    }
-		  if (i == commutative || i == commutative + 1)
-		    {
-		      int other = i + (i == commutative ? 1 : -1);
-		      operands_match[c][other]
-			= operands_match_p (recog_data.operand[c],
-					    recog_data.operand[other]);
-		    }
-		  /* Note that C is supposed to be less than I.
-		     No need to consider altering both C and I because in
-		     that case we would alter one into the other.  */
-		}
+	      if (commutative[c] != c)
+		operands_match[commutative[c]][i]
+		  = operands_match_p (recog_data.operand[commutative[c]],
+				      recog_data.operand[i]);
+	      if (commutative[i] != i && commutative[c] != c)
+		operands_match[commutative[c]][commutative[i]]
+		  = operands_match_p (recog_data.operand[commutative[c]],
+				      recog_data.operand[commutative[i]]);
 	    }
 	}
     }
 
+  /* # of loops needed to test all possible combinations of commutable operands.
+     for a single loop, keep n_permutation 0 to keep the test below easier.  */
+  n_permutation = 0;
+  if (j)
+    n_permutation = 1 << j;
+
   /* Examine each operand that is a memory reference or memory address
      and reload parts of the addresses into index registers.
      Also here any references to pseudo regs that didn't get hard regs
@@ -2755,6 +2759,8 @@ find_reloads (insn, replace, ind_levels,
 
   swapped = 0;
   goal_alternative_swapped = 0;
+  for (i = 0; i < noperands; i++)
+    commutative[i] = i;
  try_swapped:
 
   /* The constraints are made of several alternatives.
@@ -2926,13 +2932,7 @@ find_reloads (insn, replace, ind_levels,
 	  while (*p && (c = *p++) != ',')
 	    switch (c)
 	      {
-	      case '=':  case '+':  case '*':
-		break;
-
-	      case '%':
-		/* The last operand should not be marked commutative.  */
-		if (i != noperands - 1)
-		  commutative = i;
+	      case '=':  case '+':  case '*': case '%':
 		break;
 
 	      case '?':
@@ -2962,19 +2962,10 @@ find_reloads (insn, replace, ind_levels,
 		   only a single reload insn will be needed to make
 		   the two operands win.  As a result, this alternative
 		   may be rejected when it is actually desirable.)  */
-		if ((swapped && (c != commutative || i != commutative + 1))
-		    /* If we are matching as if two operands were swapped,
-		       also pretend that operands_match had been computed
-		       with swapped.
-		       But if I is the second of those and C is the first,
-		       don't exchange them, because operands_match is valid
-		       only on one side of its diagonal.  */
-		    ? (operands_match
-		       [(c == commutative || c == commutative + 1)
-		       ? 2 * commutative + 1 - c : c]
-		       [(i == commutative || i == commutative + 1)
-		       ? 2 * commutative + 1 - i : i])
-		    : operands_match[c][i])
+		/* If we are matching as if two operands were swapped,
+		   also pretend that operands_match had been computed
+		   with swapped. */
+		if (operands_match[commutative[c]][commutative[i]])
 		  {
 		    /* If we are matching a non-offsettable address where an
 		       offsettable address was expected, then we must reject
@@ -3431,11 +3422,22 @@ find_reloads (insn, replace, ind_levels,
       if (losers == 0)
 	{
 	  /* Unswap these so that they are never swapped at `finish'.  */
-	  if (commutative >= 0)
+	  if (swapped)
 	    {
-	      recog_data.operand[commutative] = substed_operand[commutative];
-	      recog_data.operand[commutative + 1]
-		= substed_operand[commutative + 1];
+	      int s = swapped;
+	      s ^= s >> 1;
+
+	      for (i = 0; s; i++, s >>= 1)
+		{
+		  register int c;
+
+		  if (!(s & 1))
+		    continue;
+		  c = commutative_op[i];
+
+		  recog_data.operand[c] = substed_operand[c];
+		  recog_data.operand[c + 1] = substed_operand[c + 1];
+		}
 	    }
 	  for (i = 0; i < noperands; i++)
 	    {
@@ -3485,52 +3487,50 @@ find_reloads (insn, replace, ind_levels,
      then we need to try each alternative twice,
      the second time matching those two operands
      as if we had exchanged them.
-     To do this, really exchange them in operands.
+     To do this, really exchange them in operands.  */
 
-     If we have just tried the alternatives the second time,
-     return operands to normal and drop through.  */
-
-  if (commutative >= 0)
+  if (++swapped <= n_permutation)
     {
-      swapped = !swapped;
-      if (swapped)
+      int s, t;
+      rtx x;
+
+      /* In every loop we just swap a single pair of operands, so that in the
+	 last loop we only need to swap a single pair back to return operands
+	 to normal and drop through.  */
+      s = swapped - 1;
+      if (swapped < n_permutation)
+	s ^= swapped;
+      s ^= s >> 1;
+
+      for (i = 0; s; i++, s >>= 1)
+	if (s & 1)
+	  break;
+      i = commutative_op[i];
+
+      t = commutative[i];
+      commutative[i] = commutative[i + 1];
+      commutative[i + 1] = t;
+
+      x = recog_data.operand[i];
+      recog_data.operand[i] = recog_data.operand[i + 1];
+      recog_data.operand[i + 1] = x;
+
+      if (swapped < n_permutation)
 	{
 	  enum reg_class tclass;
-	  int t;
 
-	  recog_data.operand[commutative] = substed_operand[commutative + 1];
-	  recog_data.operand[commutative + 1] = substed_operand[commutative];
-	  /* Swap the duplicates too.  */
-	  for (i = 0; i < recog_data.n_dups; i++)
-	    if (recog_data.dup_num[i] == commutative
-		|| recog_data.dup_num[i] == commutative + 1)
-	      *recog_data.dup_loc[i]
-		 = recog_data.operand[(int) recog_data.dup_num[i]];
-
-	  tclass = preferred_class[commutative];
-	  preferred_class[commutative] = preferred_class[commutative + 1];
-	  preferred_class[commutative + 1] = tclass;
-
-	  t = pref_or_nothing[commutative];
-	  pref_or_nothing[commutative] = pref_or_nothing[commutative + 1];
-	  pref_or_nothing[commutative + 1] = t;
+	  tclass = preferred_class[i];
+	  preferred_class[i] = preferred_class[i + 1];
+	  preferred_class[i + 1] = tclass;
+
+	  t = pref_or_nothing[i];
+	  pref_or_nothing[i] = pref_or_nothing[i + 1];
+	  pref_or_nothing[i + 1] = t;
 
 	  memcpy (constraints, recog_data.constraints,
 		  noperands * sizeof (char *));
 	  goto try_swapped;
 	}
-      else
-	{
-	  recog_data.operand[commutative] = substed_operand[commutative];
-	  recog_data.operand[commutative + 1]
-	    = substed_operand[commutative + 1];
-	  /* Unswap the duplicates too.  */
-	  for (i = 0; i < recog_data.n_dups; i++)
-	    if (recog_data.dup_num[i] == commutative
-		|| recog_data.dup_num[i] == commutative + 1)
-	      *recog_data.dup_loc[i]
-		 = recog_data.operand[(int) recog_data.dup_num[i]];
-	}
     }
 
   /* The operands don't meet the constraints.
@@ -3577,25 +3577,44 @@ find_reloads (insn, replace, ind_levels,
 
   if (goal_alternative_swapped)
     {
-      rtx tem;
+      int s = goal_alternative_swapped;
+      s ^= s >> 1;
 
-      tem = substed_operand[commutative];
-      substed_operand[commutative] = substed_operand[commutative + 1];
-      substed_operand[commutative + 1] = tem;
-      tem = recog_data.operand[commutative];
-      recog_data.operand[commutative] = recog_data.operand[commutative + 1];
-      recog_data.operand[commutative + 1] = tem;
-      tem = *recog_data.operand_loc[commutative];
-      *recog_data.operand_loc[commutative]
-	= *recog_data.operand_loc[commutative + 1];
-      *recog_data.operand_loc[commutative + 1] = tem;
-
-      for (i = 0; i < n_reloads; i++)
+      for (i = 0; s; i++, s >>= 1)
 	{
-	  if (rld[i].opnum == commutative)
-	    rld[i].opnum = commutative + 1;
-	  else if (rld[i].opnum == commutative + 1)
-	    rld[i].opnum = commutative;
+	  int c;
+	  rtx tem;
+
+	  if (!(s & 1))
+	    continue;
+	  c = commutative_op[i];
+
+	  tem = substed_operand[c];
+	  substed_operand[c] = substed_operand[c + 1];
+	  substed_operand[c + 1] = tem;
+
+	  tem = recog_data.operand[c];
+	  recog_data.operand[c] = recog_data.operand[c + 1];
+	  recog_data.operand[c + 1] = tem;
+
+	  tem = *recog_data.operand_loc[c];
+	  *recog_data.operand_loc[c] = *recog_data.operand_loc[c + 1];
+	  *recog_data.operand_loc[c + 1] = tem;
+
+	  /* Swap the duplicates too.  */
+	  for (j = 0; j < recog_data.n_dups; j++)
+	    if (recog_data.dup_num[j] == c
+		|| recog_data.dup_num[j] == c + 1)
+	      *recog_data.dup_loc[j]
+		= recog_data.operand[(int) recog_data.dup_num[j]];
+
+	  for (j = 0; j < n_reloads; j++)
+	    {
+	      if (rld[j].opnum == c)
+		rld[j].opnum = c + 1;
+	      else if (rld[j].opnum == c + 1)
+		rld[j].opnum = c;
+	    }
 	}
     }
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-04  9:29     ` Roman Zippel
@ 2002-08-04 14:46       ` Alan Modra
  2002-08-05 16:14         ` Roman Zippel
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2002-08-04 14:46 UTC (permalink / raw)
  To: Roman Zippel; +Cc: gcc, gcc-patches

On Sun, Aug 04, 2002 at 06:29:19PM +0200, Roman Zippel wrote:
> Alan Modra wrote:
> >         * recog.c (constrain_operands): Handle commutative operands.
> 
> What exactly does this patch fix?

The ICE reported at the beginning of this thread, and the following
similar one.

> What is the assembly output for this test case (register names might
> need a change):

Thanks, this testcase is useful.

void f (void)
{
  register int a asm ("%r3");
  register int b asm ("%r4");
  register int c asm ("%r5");
  register int d asm ("%r6");

  asm volatile ("addc %0, %2, %3\n\tadde %1, %4, %5"
		: "=r" (a), "=r" (b)
		: "%0" (a), "r" (c), "%1" (b), "r" (d));
  asm volatile ("addc %0, %2, %3\n\tadde %1, %4, %5"
		: "=r" (a), "=r" (b)
		: "%0" (c), "r" (a), "%1" (d), "r" (b));
}

produces, with my patch:

.f:
#APP
        addc 3, 3, 5
        adde 4, 4, 6
        addc 3, 5, 3
        adde 4, 4, 6
#NO_APP
        blr

and without:

addc.c: In function `f':
addc.c:14: error: unrecognizable insn:
(insn:HI 14 12 22 0 0x4012c0c0 (parallel [
            (set (reg/v:SI 3 r3)
                (asm_operands/v:SI ("addc %0, %2, %3
        adde %1, %4, %5") ("=r") 0 [
                        (reg/v:SI 5 r5)
                        (reg/v:SI 3 r3)
                        (reg/v:SI 4 r4)
                        (reg/v:SI 6 r6)
                    ]
                     [
                        (asm_input:SI ("%0"))
                        (asm_input:SI ("r"))
                        (asm_input:SI ("%1"))
                        (asm_input:SI ("r"))
                    ] ("addc.c") 11))
            (set (reg/v:SI 4 r4)
                (asm_operands/v:SI ("addc %0, %2, %3
        adde %1, %4, %5") ("=r") 1 [
                        (reg/v:SI 5 r5)
                        (reg/v:SI 3 r3)
                        (reg/v:SI 4 r4)
                        (reg/v:SI 6 r6)
                    ]
                     [
                        (asm_input:SI ("%0"))
                        (asm_input:SI ("r"))
                        (asm_input:SI ("%1"))
                        (asm_input:SI ("r"))
                    ] ("addc.c") 11))
        ]) -1 (insn_list 12 (nil))
    (nil))
addc.c:14: internal compiler error: Internal compiler error in reload_cse_simplify_operands, at reload1.c:8308

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-04 14:46       ` Alan Modra
@ 2002-08-05 16:14         ` Roman Zippel
  2002-08-05 16:38           ` Alan Modra
  0 siblings, 1 reply; 10+ messages in thread
From: Roman Zippel @ 2002-08-05 16:14 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc, gcc-patches

Hi,

On Mon, 5 Aug 2002, Alan Modra wrote:

> > >         * recog.c (constrain_operands): Handle commutative operands.
> >
> > What exactly does this patch fix?
>
> The ICE reported at the beginning of this thread, and the following
> similar one.

It's not fixing the problem, it's hiding it (and not very good).

> produces, with my patch:
>
> .f:
> #APP
>         addc 3, 3, 5
>         adde 4, 4, 6
>         addc 3, 5, 3
>         adde 4, 4, 6
> #NO_APP

The "3, 5, 3" line isn't correct, it's conflicting with the constraints.
I had an even better test case, which really forced reloads, when '%' was
ommitted, but it somehow got lost.
Anyway, my patch tries to find best combination of operands, e.g. for your
example it automatically would do:

asm ("addc %0, %2, %3\n\t"
     "adde %1, %4, %5"
     : "=r,r,r,r" (lo), "=r,r,r,r" (hi)
     : "r,0,r,0" (__lo), "0,r,0,r" (lo), "r,r,1,1" (__hi), "1,1,r,r" (hi))

BTW your example misses the earlyclobber constraint (&) for operand 0.

bye, Roman



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-05 16:14         ` Roman Zippel
@ 2002-08-05 16:38           ` Alan Modra
  2002-08-06 16:17             ` Roman Zippel
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2002-08-05 16:38 UTC (permalink / raw)
  To: Roman Zippel; +Cc: gcc, gcc-patches

On Tue, Aug 06, 2002 at 01:13:56AM +0200, Roman Zippel wrote:
> On Mon, 5 Aug 2002, Alan Modra wrote:
> >         addc 3, 5, 3
> 
> The "3, 5, 3" line isn't correct, it's conflicting with the constraints.

Oh?  This is from (doing some renumbering of constraints)

  asm ("addc %0, %1, %2" : "=r" (a) : "%0" (c), "r" (a));

Or expanding the "%" constraint:

  asm ("addc %0, %1, %2" : "=r,r" (a) : "0,r" (c), "r,0" (a));

Tell me why this conflicts.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-05 16:38           ` Alan Modra
@ 2002-08-06 16:17             ` Roman Zippel
  2002-08-06 17:46               ` Richard Henderson
  0 siblings, 1 reply; 10+ messages in thread
From: Roman Zippel @ 2002-08-06 16:17 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc, rth

Hi,

On Tue, 6 Aug 2002, Alan Modra wrote:

> > The "3, 5, 3" line isn't correct, it's conflicting with the constraints.
>
> Oh?  This is from (doing some renumbering of constraints)
>
>   asm ("addc %0, %1, %2" : "=r" (a) : "%0" (c), "r" (a));
>
> Or expanding the "%" constraint:
>
>   asm ("addc %0, %1, %2" : "=r,r" (a) : "0,r" (c), "r,0" (a));
>
> Tell me why this conflicts.

While thinking more about it, I think it's not really correct to exchange
the constraint, reload should only try to exchange the operands, e.g.

	asm("add %0,%2" : "=r" (a) : "%0" (b), "g" (c));

has to work as well. An example like is mentioned in the docs, the first
and second operand must be the same, otherwise you'll break some machines.
Richard, could you please confirm/correct this?

bye, Roman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: commutative asm operands
  2002-08-06 16:17             ` Roman Zippel
@ 2002-08-06 17:46               ` Richard Henderson
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2002-08-06 17:46 UTC (permalink / raw)
  To: Roman Zippel; +Cc: Alan Modra, gcc

On Wed, Aug 07, 2002 at 01:16:40AM +0200, Roman Zippel wrote:
> >   asm ("addc %0, %1, %2" : "=r" (a) : "%0" (c), "r" (a));
> >   asm ("addc %0, %1, %2" : "=r,r" (a) : "0,r" (c), "r,0" (a));
> 
> While thinking more about it, I think it's not really correct to exchange
> the constraint, reload should only try to exchange the operands, e.g.
> 
> 	asm("add %0,%2" : "=r" (a) : "%0" (b), "g" (c));
> 
> has to work as well.

Yes.

> An example like is mentioned in the docs, the first and second
> operand must be the same, otherwise you'll break some machines.

How do you mean?  If they're not the same, then reload must
rearrange them so that they are the same.  E.g.

	op0 = a = r1
	op1 = b = r3
	op2 = c = mem

one solution is 

	// don't swap
	(set r1 r3)
	(set r1 (asm_operands [r1 mem]))

another is

	// swap
	(set r1 mem)
	(set r1 (asm_operands [r1 r3]))

The one reload can't fix is if you have a matching constraint
with a memory operand, and the memories aren't the same.
Matching with a register should always work though.


r~

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2002-08-06 17:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-07-31  9:45 commutative asm operands Alan Modra
2002-07-31 22:59 ` Richard Henderson
2002-08-03 14:19 ` Roman Zippel
2002-08-03 19:45   ` Alan Modra
2002-08-04  9:29     ` Roman Zippel
2002-08-04 14:46       ` Alan Modra
2002-08-05 16:14         ` Roman Zippel
2002-08-05 16:38           ` Alan Modra
2002-08-06 16:17             ` Roman Zippel
2002-08-06 17:46               ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).