re PR middle-end/58670 (asm goto miscompilation)

	PR middle-end/58670
	* stmt.c (expand_asm_operands): Add FALLTHRU_BB argument,
	if any labels are in FALLTHRU_BB, use a special label emitted
	immediately after the asm goto insn rather than label_rtx
	of the LABEL_DECL.
	(expand_asm_stmt): Adjust caller.
	* cfgrtl.c (commit_one_edge_insertion): Force splitting of
	edge if the last insn in predecessor is a jump with single successor,
	but it isn't simplejump_p.

	* gcc.dg/torture/pr58670.c: New test.

From-SVN: r203383

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 06d5c986914c93062c50dbc01edac89d38f1f8d1..e91562fdd902ff6bc65f0bb232f8b4b5ab2d92b5 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2013-10-10  Jakub Jelinek  <jakub@redhat.com>
+
+       PR middle-end/58670
+       * stmt.c (expand_asm_operands): Add FALLTHRU_BB argument,
+       if any labels are in FALLTHRU_BB, use a special label emitted
+       immediately after the asm goto insn rather than label_rtx
+       of the LABEL_DECL.
+       (expand_asm_stmt): Adjust caller.
+       * cfgrtl.c (commit_one_edge_insertion): Force splitting of
+       edge if the last insn in predecessor is a jump with single successor,
+       but it isn't simplejump_p.
+
 2013-10-10  Richard Biener  <rguenther@suse.de>
 
        PR tree-optimization/58656

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 85738a43f36d7212e6678be46293859f339b5673..d6733a2af31e781377b8376e8387d4f9c17ed484 100644 (file)
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -1959,10 +1959,18 @@ commit_one_edge_insertion (edge e)
     }
 
   /* If the source has one successor and the edge is not abnormal,
-     insert there.  Except for the entry block.  */
+     insert there.  Except for the entry block.
+     Don't do this if the predecessor ends in a jump other than
+     unconditional simple jump.  E.g. for asm goto that points all
+     its labels at the fallthru basic block, we can't insert instructions
+     before the asm goto, as the asm goto can have various of side effects,
+     and can't emit instructions after the asm goto, as it must end
+     the basic block.  */
   else if ((e->flags & EDGE_ABNORMAL) == 0
           && single_succ_p (e->src)
-          && e->src != ENTRY_BLOCK_PTR)
+          && e->src != ENTRY_BLOCK_PTR
+          && (!JUMP_P (BB_END (e->src))
+              || simplejump_p (BB_END (e->src))))
     {
       bb = e->src;
 

diff --git a/gcc/stmt.c b/gcc/stmt.c
index c385a067e4b11cc7acf17430debd6e1ce939faee..b3fd25510abe7509ff07ef00f4f8dc47582233ca 100644 (file)
--- a/gcc/stmt.c
+++ b/gcc/stmt.c
@@ -612,6 +612,9 @@ tree_conflicts_with_clobbers_p (tree t, HARD_REG_SET *clobbered_regs)
    CLOBBERS is a list of STRING_CST nodes each naming a hard register
    that is clobbered by this insn.
 
+   LABELS is a list of labels, and if LABELS is non-NULL, FALLTHRU_BB
+   should be the fallthru basic block of the asm goto.
+
    Not all kinds of lvalue that may appear in OUTPUTS can be stored directly.
    Some elements of OUTPUTS may be replaced with trees representing temporary
    values.  The caller should copy those temporary values to the originally
@@ -621,7 +624,8 @@ tree_conflicts_with_clobbers_p (tree t, HARD_REG_SET *clobbered_regs)
 
 static void
 expand_asm_operands (tree string, tree outputs, tree inputs,
-                    tree clobbers, tree labels, int vol, location_t locus)
+                    tree clobbers, tree labels, basic_block fallthru_bb,
+                    int vol, location_t locus)
 {
   rtvec argvec, constraintvec, labelvec;
   rtx body;
@@ -642,6 +646,7 @@ expand_asm_operands (tree string, tree outputs, tree inputs,
   enum machine_mode *inout_mode = XALLOCAVEC (enum machine_mode, noutputs);
   const char **constraints = XALLOCAVEC (const char *, noutputs + ninputs);
   int old_generating_concat_p = generating_concat_p;
+  rtx fallthru_label = NULL_RTX;
 
   /* An ASM with no outputs needs to be treated as volatile, for now.  */
   if (noutputs == 0)
@@ -942,8 +947,24 @@ expand_asm_operands (tree string, tree outputs, tree inputs,
 
   /* Copy labels to the vector.  */
   for (i = 0, tail = labels; i < nlabels; ++i, tail = TREE_CHAIN (tail))
-    ASM_OPERANDS_LABEL (body, i)
-      = gen_rtx_LABEL_REF (Pmode, label_rtx (TREE_VALUE (tail)));
+    {
+      rtx r;
+      /* If asm goto has any labels in the fallthru basic block, use
+        a label that we emit immediately after the asm goto.  Expansion
+        may insert further instructions into the same basic block after
+        asm goto and if we don't do this, insertion of instructions on
+        the fallthru edge might misbehave.  See PR58670.  */
+      if (fallthru_bb
+         && label_to_block_fn (cfun, TREE_VALUE (tail)) == fallthru_bb)
+       {
+         if (fallthru_label == NULL_RTX)
+           fallthru_label = gen_label_rtx ();
+         r = fallthru_label;
+       }
+      else
+       r = label_rtx (TREE_VALUE (tail));
+      ASM_OPERANDS_LABEL (body, i) = gen_rtx_LABEL_REF (Pmode, r);
+    }
 
   generating_concat_p = old_generating_concat_p;
 
@@ -1067,6 +1088,9 @@ expand_asm_operands (tree string, tree outputs, tree inputs,
        emit_insn (body);
     }
 
+  if (fallthru_label)
+    emit_label (fallthru_label);
+
   /* For any outputs that needed reloading into registers, spill them
      back to where they belong.  */
   for (i = 0; i < noutputs; ++i)
@@ -1087,6 +1111,7 @@ expand_asm_stmt (gimple stmt)
   const char *s;
   tree str, out, in, cl, labels;
   location_t locus = gimple_location (stmt);
+  basic_block fallthru_bb = NULL;
 
   /* Meh... convert the gimple asm operands into real tree lists.
      Eventually we should make all routines work on the vectors instead
@@ -1122,6 +1147,9 @@ expand_asm_stmt (gimple stmt)
   n = gimple_asm_nlabels (stmt);
   if (n > 0)
     {
+      edge fallthru = find_fallthru_edge (gimple_bb (stmt)->succs);
+      if (fallthru)
+       fallthru_bb = fallthru->dest;
       t = labels = gimple_asm_label_op (stmt, 0);
       for (i = 1; i < n; i++)
        t = TREE_CHAIN (t) = gimple_asm_label_op (stmt, i);
@@ -1147,7 +1175,7 @@ expand_asm_stmt (gimple stmt)
 
   /* Generate the ASM_OPERANDS insn; store into the TREE_VALUEs of
      OUTPUTS some trees for where the values were actually stored.  */
-  expand_asm_operands (str, outputs, in, cl, labels,
+  expand_asm_operands (str, outputs, in, cl, labels, fallthru_bb,
                       gimple_asm_volatile_p (stmt), locus);
 
   /* Copy all the intermediate outputs into the specified outputs.  */

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 76cce59c69fa60c65439f67b22d4df1d24726516..5e4882a2430e66358e2f660560f81a93e6d6a952 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-10  Jakub Jelinek  <jakub@redhat.com>
+
+       PR middle-end/58670
+       * gcc.dg/torture/pr58670.c: New test.
+
 2013-10-09  Zhenqiang Chen  <zhenqiang.chen@arm.com>
 
        * gcc.dg/tree-ssa/phi-opt-11.c: New test.

diff --git a/gcc/testsuite/gcc.dg/torture/pr58670.c b/gcc/testsuite/gcc.dg/torture/pr58670.c
new file mode 100644 (file)
index 0000000..e4536cc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr58670.c
@@ -0,0 +1,47 @@
+/* PR middle-end/58670 */
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+
+#if defined (__i386__) || defined (__x86_64__)
+#define ASM_STR "bts $1, %0; jc %l[lab]"
+#endif
+
+__attribute__((noinline, noclone)) int
+foo (int a, int b)
+{
+  if (a)
+    return -3;
+#ifdef ASM_STR
+  asm volatile goto (ASM_STR : : "m" (b) : "memory" : lab);
+  return 0;
+lab:
+#endif
+  return 0;
+}
+
+int
+bar (int a, int b)
+{
+  if (a)
+    return -3;
+#ifdef ASM_STR
+  asm volatile goto (ASM_STR : : "m" (b) : "memory" : lab);
+  return 0;
+lab:
+#endif
+  return 0;
+}
+
+int
+main ()
+{
+  if (foo (1, 0) != -3
+      || foo (0, 3) != 0
+      || foo (1, 0) != -3
+      || foo (0, 0) != 0
+      || bar (1, 0) != -3
+      || bar (0, 3) != 0
+      || bar (1, 0) != -3
+      || bar (0, 0) != 0)
+    __builtin_abort ();
+  return 0;
+}