diff mbox

Fix assembly dialect handling in asm_fprintf

Message ID 20120725204029.5ccf50b8@spoyarek
State New
Headers show

Commit Message

Siddhesh Poyarekar July 25, 2012, 3:10 p.m. UTC
Hi,

While reviewing code to fix a previous patch submission around assembly
dialect handling[1], I found that the dialect handling in asm_fprintf
is different from that in output_asm_insn and it might be broken too,
since encountering a '|' simply leads to skipping all of the string
until a '}' is encountered, instead of printing it if it is not part
of the assembly dialect syntax. This is just never hit with user code
since asm_fprintf is used only internally.

Attached patch moves the asm dialect handling into a separate function
that both output_asm_insn and asm_fprintf call. I have also removed the
extra:

             if (*p == '|')
               p++;

in the '{' case, since it is unnecessary. In fact it will behave
incorrectly if a target has more than two dialects (none of the targets
have that currently) since something like this:

foo||bar

will incorrectly give 'bar' for dialect 1 and nothing for dialect 2
(and of course, foo for dialect 0) instead of nothing for dialect 1
and bar for dialect 2.

I have verified that this does not cause a regression in the testsuite
on x86_64. There is also an additional test case in this patch that
verifies that the double '|' bug I mentioned above is fixed.

Regards,
Siddhesh

[1] http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00840.html

gcc/ChangeLog:

2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* final.c [ASSEMBLER_DIALECT](do_assembler_dialects): New
	function to implement assembler dialects.
	(output_asm_insn): Use do_assembler_dialects.
	(asm_fprintf): Likewise.

testsuite/ChangeLog:

2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* gcc.dg/asm-dialect-1.c: New test case.

Comments

Richard Henderson July 25, 2012, 3:31 p.m. UTC | #1
On 07/25/2012 08:10 AM, Siddhesh Poyarekar wrote:
> 2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* final.c [ASSEMBLER_DIALECT](do_assembler_dialects): New
> 	function to implement assembler dialects.
> 	(output_asm_insn): Use do_assembler_dialects.
> 	(asm_fprintf): Likewise.

Ok ...

> testsuite/ChangeLog:
> 
> 2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* gcc.dg/asm-dialect-1.c: New test case.

... except this should go in gcc.target/i386/ without the { target } qualifier.


r~
diff mbox

Patch

diff --git a/gcc/final.c b/gcc/final.c
index 7db0471..095d608 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -3338,6 +3338,72 @@  output_asm_operand_names (rtx *operands, int *oporder, int nops)
     }
 }
 
+#ifdef ASSEMBLER_DIALECT
+/* Helper function to parse assembler dialects in the asm string.
+   This is called from output_asm_insn and asm_fprintf.  */
+static const char *
+do_assembler_dialects (const char *p, int *dialect)
+{
+  char c = *(p - 1);
+
+  switch (c)
+    {
+    case '{':
+      {
+        int i;
+
+        if (*dialect)
+          output_operand_lossage ("nested assembly dialect alternatives");
+        else
+          *dialect = 1;
+
+        /* If we want the first dialect, do nothing.  Otherwise, skip
+           DIALECT_NUMBER of strings ending with '|'.  */
+        for (i = 0; i < dialect_number; i++)
+          {
+            while (*p && *p != '}' && *p++ != '|')
+	      ;
+            if (*p == '}')
+	      break;
+          }
+
+        if (*p == '\0')
+          output_operand_lossage ("unterminated assembly dialect alternative");
+      }
+      break;
+
+    case '|':
+      if (*dialect)
+        {
+          /* Skip to close brace.  */
+          do
+            {
+	      if (*p == '\0')
+		{
+		  output_operand_lossage ("unterminated assembly dialect alternative");
+		  break;
+		}
+            }
+          while (*p++ != '}');
+          *dialect = 0;
+        }
+      else
+        putc (c, asm_out_file);
+      break;
+
+    case '}':
+      if (! *dialect)
+        putc (c, asm_out_file);
+      *dialect = 0;
+      break;
+    default:
+      gcc_unreachable ();
+    }
+
+  return p;
+}
+#endif
+
 /* Output text from TEMPLATE to the assembler output file,
    obeying %-directions to substitute operands taken from
    the vector OPERANDS.
@@ -3404,54 +3470,9 @@  output_asm_insn (const char *templ, rtx *operands)
 
 #ifdef ASSEMBLER_DIALECT
       case '{':
-	{
-	  int i;
-
-	  if (dialect)
-	    output_operand_lossage ("nested assembly dialect alternatives");
-	  else
-	    dialect = 1;
-
-	  /* If we want the first dialect, do nothing.  Otherwise, skip
-	     DIALECT_NUMBER of strings ending with '|'.  */
-	  for (i = 0; i < dialect_number; i++)
-	    {
-	      while (*p && *p != '}' && *p++ != '|')
-		;
-	      if (*p == '}')
-		break;
-	      if (*p == '|')
-		p++;
-	    }
-
-	  if (*p == '\0')
-	    output_operand_lossage ("unterminated assembly dialect alternative");
-	}
-	break;
-
-      case '|':
-	if (dialect)
-	  {
-	    /* Skip to close brace.  */
-	    do
-	      {
-		if (*p == '\0')
-		  {
-		    output_operand_lossage ("unterminated assembly dialect alternative");
-		    break;
-		  }
-	      }
-	    while (*p++ != '}');
-	    dialect = 0;
-	  }
-	else
-	  putc (c, asm_out_file);
-	break;
-
       case '}':
-	if (! dialect)
-	  putc (c, asm_out_file);
-	dialect = 0;
+      case '|':
+	p = do_assembler_dialects (p, &dialect);
 	break;
 #endif
 
@@ -3910,6 +3931,9 @@  asm_fprintf (FILE *file, const char *p, ...)
 {
   char buf[10];
   char *q, c;
+#ifdef ASSEMBLER_DIALECT
+  int dialect = 0;
+#endif
   va_list argptr;
 
   va_start (argptr, p);
@@ -3921,29 +3945,9 @@  asm_fprintf (FILE *file, const char *p, ...)
       {
 #ifdef ASSEMBLER_DIALECT
       case '{':
-	{
-	  int i;
-
-	  /* If we want the first dialect, do nothing.  Otherwise, skip
-	     DIALECT_NUMBER of strings ending with '|'.  */
-	  for (i = 0; i < dialect_number; i++)
-	    {
-	      while (*p && *p++ != '|')
-		;
-
-	      if (*p == '|')
-		p++;
-	    }
-	}
-	break;
-
-      case '|':
-	/* Skip to close brace.  */
-	while (*p && *p++ != '}')
-	  ;
-	break;
-
       case '}':
+      case '|':
+	p = do_assembler_dialects (p, &dialect);
 	break;
 #endif
 
diff --git a/gcc/testsuite/gcc.dg/asm-dialect-1.c b/gcc/testsuite/gcc.dg/asm-dialect-1.c
new file mode 100644
index 0000000..b3da004
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-dialect-1.c
@@ -0,0 +1,16 @@ 
+/* { dg-do run { target *86*-*-* } } */
+/* { dg-options "-masm=intel" } */
+
+extern void abort (void);
+
+int
+main (void)
+{
+  int f = 0;
+  asm ("{movl $42, %%eax | mov eax, 42}" : :);
+  asm ("{movl $41, %0||mov %0, 43}" : "=r"(f));
+  if (f != 42)
+    abort ();
+
+  return 0;
+}