PATCH RFA: Fix for PR inline-asm/6162

Ian Lance Taylor ian@wasabisystems.com
Thu Jan 29 03:11:00 GMT 2004


PR inline-asm/6162 is about an ICE when compiling code which uses an
inline assembler instruction with two pairs of commutative operands.
As discussed in the PR, this is not really supported by reload.
Reload will happily process such an insn, but it won't produce the
right result, because it can only keep track of one pair of
commutative operands.

The gcc documentation now prohibits more than one pair of commutative
operands.  We could also easily detect this case in the code.
However, there is apparently a fair amount of existing code which uses
more than one pair of commutative operands, including code in gcc's
longlong.h file.  So it seems better to let this case pass in inline
assembler code, but to just handle one of the pairs of commutative
operands and not get confused by the others.

This patch implements that, only paying attention to the first pair of
commutative operands seen in an instruction.

As a happy benefit, with this patch, the test case in the PR compiles.
Ignoring the second pair of commutative operands forces reload to do
some register shuffling, and the result works.

I've tested this by running the testsuite and doing a bootstrap
(without Ada) on i686-pc-linux-gnu.

OK for mainline?

Ian


2004-01-28  Ian Lance Taylor  <ian@wasabisystems.com>

	* reload.c (find_reloads): Only support one pair of commutative
	operands.


Index: reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.230
diff -p -u -r1.230 reload.c
--- reload.c	21 Jan 2004 20:40:03 -0000	1.230
+++ reload.c	29 Jan 2004 03:00:03 -0000
@@ -2605,7 +2605,17 @@ find_reloads (rtx insn, int replace, int
 	      if (i == noperands - 1)
 		abort ();
 
-	      commutative = i;
+	      /* We currently only support one commutative pair of
+		 operands.  Some existing asm code currently uses more
+		 than one pair.  Previously, that would usually work,
+		 but sometimes it would crash the compiler.  We
+		 continue supporting that case as well as we can by
+		 silently ignoring all but the first pair.  In the
+		 future we may handle it correctly.  */
+	      if (commutative < 0)
+		commutative = i;
+	      else if (!this_insn_is_asm)
+		abort ();
 	    }
 	  else if (ISDIGIT (c))
 	    {
@@ -2979,9 +2989,8 @@ find_reloads (rtx insn, int replace, int
 		break;
 
 	      case '%':
-		/* The last operand should not be marked commutative.  */
-		if (i != noperands - 1)
-		  commutative = i;
+		/* We only support one commutative marker, the first
+		   one.  We already set commutative above.  */
 		break;
 
 	      case '?':



More information about the Gcc-patches mailing list