Patchwork Fix PR 54362 (COND_EXPR not understood by ITM)

login
register
mail settings
Submitter Andrew Pinski
Date Sept. 4, 2012, 7:20 a.m.
Message ID <CA+=Sn1mfOdPEce2+MSZJs=T=5Tw=zJ5+nVxhGyk_=-VE9GdTYg@mail.gmail.com>
Download mbox | patch
Permalink /patch/181506/
State New
Headers show

Comments

Andrew Pinski - Sept. 4, 2012, 7:20 a.m.
Hi,
  The problem here is that trans-mem.c does not take into account that
COND_EXPR can happen for pointers.  This patch modifies
thread_private_new_memory to handle COND_EXPR as it can handle PHI
nodes.  The testcase is a modified version of memopt-12.c but with a
loop which both LIM and if-convert can change the conditional to a
COND_EXPR.

I found this problem when I was producing a pass which does a full
if-convert before expanding (well changing the last phi-opt pass) and
it produces COND_EXPRs and memopt-12.c started to fail.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

Thanks,
Andrew Pinski

ChangeLog:
* trans-mem.c (thread_private_new_memory): Handle COND_EXPR also.

testsuite/ChangeLog:
* gcc.dg/tm/memopt-16.c: New testcase.
Andrew Pinski - Sept. 11, 2012, 4:20 a.m.
On Tue, Sep 4, 2012 at 12:20 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> Hi,
>   The problem here is that trans-mem.c does not take into account that
> COND_EXPR can happen for pointers.  This patch modifies
> thread_private_new_memory to handle COND_EXPR as it can handle PHI
> nodes.  The testcase is a modified version of memopt-12.c but with a
> loop which both LIM and if-convert can change the conditional to a
> COND_EXPR.
>
> I found this problem when I was producing a pass which does a full
> if-convert before expanding (well changing the last phi-opt pass) and
> it produces COND_EXPRs and memopt-12.c started to fail.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.


Applied after approval from RTH offline.

Thanks,
Andrew

>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * trans-mem.c (thread_private_new_memory): Handle COND_EXPR also.
>
> testsuite/ChangeLog:
> * gcc.dg/tm/memopt-16.c: New testcase.

Patch

Index: testsuite/gcc.dg/tm/memopt-16.c
===================================================================
--- testsuite/gcc.dg/tm/memopt-16.c	(revision 0)
+++ testsuite/gcc.dg/tm/memopt-16.c	(revision 0)
@@ -0,0 +1,43 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O3 -fdump-tree-tmmark" } */
+/* Like memopt-12.c but the phi is inside a look which causes
+   it to be converted into a COND_EXPR.  */
+
+extern int test(void) __attribute__((transaction_safe));
+extern void *malloc (__SIZE_TYPE__) __attribute__((malloc,transaction_safe));
+
+struct large { int foo[500]; };
+
+int f(int j)
+{
+  int *p1, *p2, *p3;
+
+  p1 = malloc (sizeof (*p1)*5000);
+  __transaction_atomic {
+    _Bool t;
+    int i = 1;
+    *p1 = 0;
+
+    p2 = malloc (sizeof (*p2)*6000);
+    *p2 = 1;
+    t = test();
+
+    for (i = 0;i < j;i++)
+    {
+
+    /* p3 = PHI (p1, p2) */
+    if (t)
+      p3 = p1;
+    else
+      p3 = p2;
+
+    /* Since both p1 and p2 are thread-private, we can inherit the
+       logging already done.  No ITM_W* instrumentation necessary.  */
+    *p3 = 555;
+    }
+  }
+  return p3[something()];
+}
+
+/* { dg-final { scan-tree-dump-times "ITM_WU" 0 "tmmark" } } */
+/* { dg-final { cleanup-tree-dump "tmmark" } } */
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 190908)
+++ trans-mem.c	(working copy)
@@ -1379,6 +1379,19 @@  thread_private_new_memory (basic_block e
 	  /* x = (cast*) foo ==> foo */
 	  else if (code == VIEW_CONVERT_EXPR || code == NOP_EXPR)
 	    x = gimple_assign_rhs1 (stmt);
+	  /* x = c ? op1 : op2 == > op1 or op2 just like a PHI */
+	  else if (code == COND_EXPR)
+	    {
+	      tree op1 = gimple_assign_rhs2 (stmt);
+	      tree op2 = gimple_assign_rhs3 (stmt);
+	      enum thread_memory_type mem;
+	      retval = thread_private_new_memory (entry_block, op1);
+	      if (retval == mem_non_local)
+		goto new_memory_ret;
+	      mem = thread_private_new_memory (entry_block, op2);
+	      retval = MIN (retval, mem);
+	      goto new_memory_ret;
+	    }
 	  else
 	    {
 	      retval = mem_non_local;