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 <[email protected]>
* 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 <[email protected]>
* gcc.dg/asm-dialect-1.c: New test case.
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;
+}