This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix ssa coalescing with inline asm (PR middle-end/70593)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>, Jeff Law <law at redhat dot com>, Bernd Schmidt <bschmidt at redhat dot com>, Alexandre Oliva <aoliva at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 8 Apr 2016 16:54:22 +0200
- Subject: [PATCH] Fix ssa coalescing with inline asm (PR middle-end/70593)
- Authentication-results: sourceware.org; auth=none
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
xen is miscompiled since Alex' SSA coalescing changes.
The problem is that we have there inline asm that sets more than one
SSA_NAME, one of them is dead though (has zero uses) and because of
the zero uses coalescing doesn't see any conflicts and puts both
the SSA_NAMEs in the two GIMPLE_ASM outputs into the same partition.
During expansion we then emit the ASM_OPERANDS followed by copying of the
two outputs into DECL_RTL of their SSA_NAMEs. But as both are in the same
partition, that is the same pseudo. The first one is what we want,
and the second one is for the zero uses SSA_NAME, which then overwrites
the right one with unrelated value.
(insn 9 6 7 2 (parallel [
(set (reg:DI 89 [ c ])
(asm_operands/v:DI ("xorl %k1, %k1
movl $7, %k0") ("=c") 0 [
(reg/v:DI 87 [ <retval> ])
(reg/v:DI 87 [ <retval> ])
]
[
(asm_input:DI ("0") pr70593.c:9)
(asm_input:DI ("1") pr70593.c:9)
]
[] pr70593.c:9))
(set (reg:DI 90 [ a ])
(asm_operands/v:DI ("xorl %k1, %k1
movl $7, %k0") ("=a") 1 [
(reg/v:DI 87 [ <retval> ])
(reg/v:DI 87 [ <retval> ])
]
[
(asm_input:DI ("0") pr70593.c:9)
(asm_input:DI ("1") pr70593.c:9)
]
[] pr70593.c:9))
(clobber (mem:BLK (scratch) [0 A8]))
(clobber (reg:CCFP 18 fpsr))
(clobber (reg:CC 17 flags))
]) pr70593.c:9 -1
(nil))
(insn 7 9 8 2 (set (reg/v:DI 87 [ <retval> ])
(reg:DI 89 [ c ])) pr70593.c:9 -1
(nil))
(insn 8 7 13 2 (set (reg/v:DI 87 [ <retval> ])
(reg:DI 90 [ a ])) pr70593.c:9 -1
(nil))
The following patch handles this by making sure we record conflicts
between multiple SSA_NAME outputs of the GIMPLE_ASM, even when some of them
are not live, by emulating what really happens during expansion - that
they are live in the moves after the asm insn. The
live_track_process_def calls later on remove it again from the live
partitions and thus undo the live_track_process_use effect, so all
the patch changes is that during the processing of e.g. the first
SSA_NAME output it sees the second (and perhaps third ...) SSA_NAME output
as live and adds conflict, similarly when processing the second one
if there are more than two, it again sees the third one and adds conflict
etc.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-04-08 Jakub Jelinek <jakub@redhat.com>
PR middle-end/70593
* tree-ssa-coalesce.c (build_ssa_conflict_graph): For GIMPLE_ASM
with multiple SSA_NAME defs, force the outputs other than first
to be live before calling live_track_process_def on each output.
* gcc.target/i386/pr70593.c: New test.
--- gcc/tree-ssa-coalesce.c.jj 2016-03-30 16:00:17.000000000 +0200
+++ gcc/tree-ssa-coalesce.c 2016-04-08 12:36:26.409403204 +0200
@@ -905,6 +905,27 @@ build_ssa_conflict_graph (tree_live_info
}
else if (is_gimple_debug (stmt))
continue;
+ else if (gimple_code (stmt) == GIMPLE_ASM
+ && gimple_asm_noutputs (as_a <gasm *> (stmt)) > 1)
+ {
+ /* For GIMPLE_ASM as the only statement which can have
+ more than one SSA_NAME definition, pretend all the
+ SSA_NAME outputs but the first one are live at this point,
+ so that conflicts are added in between all those even
+ when they are actually not really live after the asm,
+ because expansion might copy those into pseudos after
+ the asm and if multiple outputs share the same partition,
+ it might overwrite those that should be live. E.g.
+ asm volatile (".." : "=r" (a) : "=r" (b) : "0" (a), "1" (a));
+ return a;
+ See PR70593. */
+ bool first = true;
+ FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_DEF)
+ if (first)
+ first = false;
+ else
+ live_track_process_use (live, var);
+ }
FOR_EACH_SSA_TREE_OPERAND (var, stmt, iter, SSA_OP_DEF)
live_track_process_def (live, var, graph);
--- gcc/testsuite/gcc.target/i386/pr70593.c.jj 2016-04-08 12:59:27.352278471 +0200
+++ gcc/testsuite/gcc.target/i386/pr70593.c 2016-04-08 12:48:27.000000000 +0200
@@ -0,0 +1,19 @@
+/* PR middle-end/70593 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+__attribute__((noinline, noclone)) unsigned long
+foo (unsigned x)
+{
+ unsigned long a, c = x;
+ asm volatile ("xorl\t%k1, %k1\n\tmovl\t$7, %k0" : "=c" (c), "=a" (a) : "0" (c), "1" (c) : "memory");
+ return c;
+}
+
+int
+main ()
+{
+ if (foo (3) != 7)
+ __builtin_abort ();
+ return 0;
+}
Jakub