diff mbox

[ARM] PR target/78694: Avoid invalid RTL sharing in minipool code

Message ID 584AB9B1.2070004@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Dec. 9, 2016, 2:03 p.m. UTC
Hi all,

In this ICE GCC reports invalid RTL sharing in the pattern:
(insn 955 954 956 (unspec_volatile [
             (const:SI (unspec:SI [
                         (symbol_ref:SI ("a") [flags 0xe8] <var_decl 0x7fbb0d37cbd0 a>)
                         (const_int 4 [0x4])
                     ] UNSPEC_TLS))
         ] VUNSPEC_POOL_4) -1
      (nil))
seb.c:116:1: error: shared rtx
(const:SI (unspec:SI [
             (symbol_ref:SI ("a") [flags 0xe8] <var_decl 0x7fbb0d37cbd0 a>)
             (const_int 4 [0x4])
         ] UNSPEC_TLS))

This is the consttable_4 pattern emitted by dump_minipool and the shared rtx that's causing
the problem is the operand to that pattern that comes from the 'value' field of Mnode.

I'm not very familiar with the exact restrictions on sharing RTL but I believe the right way
to fix these is to do a copy_rtx of the value we want to use. So this patch does that when the
pool entries are emitted.

The ICE is quite fragile. See the PR for more details, but I've been able to reliably reproduce it
on an arm-none-linux-gnueabihf target with -mcpu=arm7tdmi -mfloat-abi=soft -mfpu=vfp -mthumb and this
patch fixes that there.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/78694
     * config/arm/arm.c (dump_minipool): Copy mp->value before emitting it
     in the minipool to avoid invalid RTL sharing.

2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/78694
     * gcc.c-torture/compile/pr78694.c: New test.

Comments

Kyrill Tkachov Dec. 16, 2016, 3:20 p.m. UTC | #1
Ping.
https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00849.html

Thanks,
Kyrill

On 09/12/16 14:03, Kyrill Tkachov wrote:
> Hi all,

>

> In this ICE GCC reports invalid RTL sharing in the pattern:

> (insn 955 954 956 (unspec_volatile [

>             (const:SI (unspec:SI [

>                         (symbol_ref:SI ("a") [flags 0xe8] <var_decl 0x7fbb0d37cbd0 a>)

>                         (const_int 4 [0x4])

>                     ] UNSPEC_TLS))

>         ] VUNSPEC_POOL_4) -1

>      (nil))

> seb.c:116:1: error: shared rtx

> (const:SI (unspec:SI [

>             (symbol_ref:SI ("a") [flags 0xe8] <var_decl 0x7fbb0d37cbd0 a>)

>             (const_int 4 [0x4])

>         ] UNSPEC_TLS))

>

> This is the consttable_4 pattern emitted by dump_minipool and the shared rtx that's causing

> the problem is the operand to that pattern that comes from the 'value' field of Mnode.

>

> I'm not very familiar with the exact restrictions on sharing RTL but I believe the right way

> to fix these is to do a copy_rtx of the value we want to use. So this patch does that when the

> pool entries are emitted.

>

> The ICE is quite fragile. See the PR for more details, but I've been able to reliably reproduce it

> on an arm-none-linux-gnueabihf target with -mcpu=arm7tdmi -mfloat-abi=soft -mfpu=vfp -mthumb and this

> patch fixes that there.

>

> Bootstrapped and tested on arm-none-linux-gnueabihf.

>

> Ok for trunk?

>

> Thanks,

> Kyrill

>

> 2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/78694

>     * config/arm/arm.c (dump_minipool): Copy mp->value before emitting it

>     in the minipool to avoid invalid RTL sharing.

>

> 2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/78694

>     * gcc.c-torture/compile/pr78694.c: New test.
Sebastian Huber Dec. 20, 2016, 9:24 a.m. UTC | #2
Hello,

it would be quite nice if someone could have a look at this since this 
breaks the GCC build with libgomp enabled for all Thumb-1 targets.

On 16/12/16 16:20, Kyrill Tkachov wrote:
> Ping.

> https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00849.html

>

> Thanks,

> Kyrill


-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Ramana Radhakrishnan Dec. 20, 2016, 9:34 a.m. UTC | #3
On 09/12/16 14:03, Kyrill Tkachov wrote:
> Hi all,

>

> In this ICE GCC reports invalid RTL sharing in the pattern:

> (insn 955 954 956 (unspec_volatile [

>             (const:SI (unspec:SI [

>                         (symbol_ref:SI ("a") [flags 0xe8] <var_decl

> 0x7fbb0d37cbd0 a>)

>                         (const_int 4 [0x4])

>                     ] UNSPEC_TLS))

>         ] VUNSPEC_POOL_4) -1

>      (nil))

> seb.c:116:1: error: shared rtx

> (const:SI (unspec:SI [

>             (symbol_ref:SI ("a") [flags 0xe8] <var_decl 0x7fbb0d37cbd0 a>)

>             (const_int 4 [0x4])

>         ] UNSPEC_TLS))

>

> This is the consttable_4 pattern emitted by dump_minipool and the shared

> rtx that's causing

> the problem is the operand to that pattern that comes from the 'value'

> field of Mnode.

>

> I'm not very familiar with the exact restrictions on sharing RTL but I

> believe the right way

> to fix these is to do a copy_rtx of the value we want to use. So this

> patch does that when the

> pool entries are emitted.

>

> The ICE is quite fragile. See the PR for more details, but I've been

> able to reliably reproduce it

> on an arm-none-linux-gnueabihf target with -mcpu=arm7tdmi

> -mfloat-abi=soft -mfpu=vfp -mthumb and this

> patch fixes that there.

>

> Bootstrapped and tested on arm-none-linux-gnueabihf.

>

> Ok for trunk?


Sorry about the time it's taken to review this - I'm way behind on my 
gcc-patches mailbox.

Ok.

Thanks,
Ramana
>

> Thanks,

> Kyrill

>

> 2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/78694

>     * config/arm/arm.c (dump_minipool): Copy mp->value before emitting it

>     in the minipool to avoid invalid RTL sharing.

>

> 2016-12-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>     PR target/78694

>     * gcc.c-torture/compile/pr78694.c: New test.
diff mbox

Patch

commit 947b7660395c31e79605db30c4a1e306a4b050bf
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Dec 8 11:45:46 2016 +0000

    [ARM] PR target/78694: Avoid invalid RTL sharing in minipool code

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 437da6f..6fcc23f 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -15959,35 +15959,37 @@  dump_minipool (rtx_insn *scan)
 	      fputc ('\n', dump_file);
 	    }
 
+	  rtx val = copy_rtx (mp->value);
+
 	  switch (GET_MODE_SIZE (mp->mode))
 	    {
 #ifdef HAVE_consttable_1
 	    case 1:
-	      scan = emit_insn_after (gen_consttable_1 (mp->value), scan);
+	      scan = emit_insn_after (gen_consttable_1 (val), scan);
 	      break;
 
 #endif
 #ifdef HAVE_consttable_2
 	    case 2:
-	      scan = emit_insn_after (gen_consttable_2 (mp->value), scan);
+	      scan = emit_insn_after (gen_consttable_2 (val), scan);
 	      break;
 
 #endif
 #ifdef HAVE_consttable_4
 	    case 4:
-	      scan = emit_insn_after (gen_consttable_4 (mp->value), scan);
+	      scan = emit_insn_after (gen_consttable_4 (val), scan);
 	      break;
 
 #endif
 #ifdef HAVE_consttable_8
 	    case 8:
-	      scan = emit_insn_after (gen_consttable_8 (mp->value), scan);
+	      scan = emit_insn_after (gen_consttable_8 (val), scan);
 	      break;
 
 #endif
 #ifdef HAVE_consttable_16
 	    case 16:
-              scan = emit_insn_after (gen_consttable_16 (mp->value), scan);
+              scan = emit_insn_after (gen_consttable_16 (val), scan);
               break;
 
 #endif
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr78694.c b/gcc/testsuite/gcc.c-torture/compile/pr78694.c
new file mode 100644
index 0000000..bc31944
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr78694.c
@@ -0,0 +1,118 @@ 
+/* PR target/78694.  */
+
+enum
+{
+  MEMMODEL_RELAXED,
+  MEMMODEL_ACQUIRE,
+  PRIORITY_INSERT_END
+};
+enum
+{
+  PQ_CHILDREN,
+  PQ_TASKGROUP
+};
+struct gomp_team_state
+{
+  struct gomp_team *team;
+};
+enum gomp_task_kind
+{
+  GOMP_TASK_UNDEFERRED,
+  GOMP_TASK_WAITING
+};
+struct gomp_taskwait
+{
+  _Bool in_taskwait;
+};
+struct gomp_task
+{
+  struct gomp_task *parent;
+  int children_queue;
+  struct gomp_taskgroup *taskgroup;
+  int dependers;
+  struct gomp_taskwait taskwait;
+  enum gomp_task_kind kind;
+  _Bool in_tied_task;
+} j, q, *n;
+struct gomp_taskgroup
+{
+  _Bool in_taskgroup_wait;
+  int num_children;
+} l;
+struct gomp_team
+{
+  int task_queue;
+  int task_running_count;
+};
+struct gomp_thread
+{
+  struct gomp_team_state ts;
+  struct gomp_task task;
+} extern __thread a;
+
+int b, c, d, e, f, g, h, i, k, m, o, p, r;
+
+void priority_queue_next_task (struct gomp_task *, int, int);
+int gomp_task_run_pre (struct gomp_task *, struct gomp_task, struct gomp_team);
+void priority_queue_insert (int, struct gomp_task);
+void priority_queue_insert2 (int, struct gomp_task, int, int, int);
+void priority_queue_insert3 (int, struct gomp_task, int, int, int);
+void gomp_sem_post (int);
+void free (void *);
+
+_Bool s;
+int
+GOMP_taskgroup_end ()
+{
+  struct gomp_thread *t = &a;
+  struct gomp_team u = *t->ts.team;
+  struct gomp_task *v = &t->task, *w;
+  if (__atomic_load_n (&l.num_children, MEMMODEL_ACQUIRE))
+    while (1)
+      {
+	if (l.num_children)
+	  priority_queue_next_task (v, u.task_queue, r);
+	else if (w)
+	  free (w);
+	if (n->kind == GOMP_TASK_WAITING)
+	  {
+	    s = gomp_task_run_pre (n, q, u);
+	    if (__builtin_expect (s, 0))
+	      {
+		if (w)
+		  free (w);
+		goto finish_cancelled;
+	      }
+	    n = 0;
+	    l.in_taskgroup_wait = 1;
+	  }
+	if (w)
+	  {
+	    t->task = *n;
+	    if (__builtin_expect (p, 0))
+	      if (o)
+		t->task = *v;
+	  }
+	if (n)
+	  {
+	    struct gomp_task x = x;
+	    for (; i; b++)
+	      {
+		struct gomp_task y = j;
+		if (g)
+		  continue;
+		priority_queue_insert (PQ_CHILDREN, x);
+		if (x.taskwait.in_taskwait)
+		  priority_queue_insert2 (PQ_TASKGROUP, y, e, 0, d);
+		if (h)
+		  gomp_sem_post (f);
+		priority_queue_insert3 (k, y, PRIORITY_INSERT_END, 0, d);
+		++c;
+	      }
+	  }
+      finish_cancelled:
+	w = (struct gomp_task *) (n - u.task_running_count - v);
+      }
+  v->taskgroup = (struct gomp_taskgroup *) m;
+  return 1;
+}