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  <ja...@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

Reply via email to