Patchwork PR middle-end/53850: memset builtin problem in TM

login
register
mail settings
Submitter Aldy Hernandez
Date Sept. 21, 2012, 9 p.m.
Message ID <505CD56A.3040608@redhat.com>
Download mbox | patch
Permalink /patch/185931/
State New
Headers show

Comments

Aldy Hernandez - Sept. 21, 2012, 9 p.m.
The problem here is that a simple loop gets transformed into a 
__builtin_memset by the loop distribution pass:

+  __transaction_atomic
+  {
+    for (i = 0; i < 100; ++i)
+      pp[i] = 0x33;
+  }

Since this pass occurs after IPA-tm, the new call to __builtin_memset 
hasn't been analyzed so it has no cgraph node associated with it.

Barring major surgery to the entire TM infrastructure, I think the 
easiest thing to do is find the memset replacement late in 
expand_call_tm and create a cgraph node there.

I also experimented with generating the __builtin_TM_memset in the loop 
distribution pass, but that seemed overly kludgy, as every pass adding 
calls after IPA passes would have to have intimate knowledge of the TM 
builtins.  The patch below seemed like a better idea.

Tested on x86-64 Linux.

OK?

p.s. What do you think of the tm_may_enter_irr comment below?
PR middle-end/53850
	* trans-mem.c (expand_call_tm): Handle late built built-ins.
Richard Henderson - Sept. 22, 2012, 1:32 a.m.
On 09/21/2012 02:00 PM, Aldy Hernandez wrote:
> +	  /* ?? For TM_* builtin replacements, can we set this to FALSE??
> +	     Otherwise, do we need to propagate the may_irr bit?  */
> +	  node->local.tm_may_enter_irr = true;

Yes we can.  Indeed, I think we should have to insist on it.

Assert that the replacement is one of our TM builtins.  If we're
replacing anything else at this stage, something (else) bad has happened.


r~

Patch

diff --git a/gcc/testsuite/gcc.dg/tm/pr53850.c b/gcc/testsuite/gcc.dg/tm/pr53850.c
new file mode 100644
index 0000000..ca2c604
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tm/pr53850.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O3" } */
+
+unsigned char pp[100];
+
+void
+foo (void)
+{
+  int i;
+  __transaction_atomic
+  {
+    for (i = 0; i < 100; ++i)
+      pp[i] = 0x33;
+  }
+}
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 242b470..4965b3d 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -2296,8 +2296,27 @@  expand_call_tm (struct tm_region *region,
     }
 
   node = cgraph_get_node (fn_decl);
-  /* All calls should have cgraph here. */
-  gcc_assert (node);
+  /* All calls should have cgraph here.  */
+  if (!node)
+    {
+      /* We can have a nodeless call here if some pass after IPA-tm
+	 added uninstrumented calls.  For example, loop distribution
+	 can transform certain loop constructs into __builtin_mem*
+	 calls.  In this case, see if we have a suitable TM
+	 replacement and fill in the gaps.  */
+      tree repl = find_tm_replacement_function (fn_decl);
+      if (repl)
+	{
+	  gimple_call_set_fndecl (stmt, repl);
+	  update_stmt (stmt);
+	  node = cgraph_create_node (repl);
+	  /* ?? For TM_* builtin replacements, can we set this to FALSE??
+	     Otherwise, do we need to propagate the may_irr bit?  */
+	  node->local.tm_may_enter_irr = true;
+	  return expand_call_tm (region, gsi);
+	}
+      gcc_unreachable ();
+    }
   if (node->local.tm_may_enter_irr)
     transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE);