diff mbox

[trans-mem] PR46567 again

Message ID 4D5549F6.7070008@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 11, 2011, 2:38 p.m. UTC 
In tm_ipa_execute we iterate over the irr_worklist until no more work is 
left, which means, that ipa_tm_propagate_irr() can be called multiple 
for a given function.

#ifdef __VERBOSE
In the testcase below, we analyze readLoop(), noting that the call to 
funky() will go irrevocable, but since we have no hard evidence on 
SeqfileGetLine(), we don't know its BB will also go irrevocable.

Then we analyze SeqfileGetLine(), at which point we know it will go 
irrevocable, and we proceed to add its callers (readLoop() again) to the 
worklist.

When we analyze readLoop() again, we notice right away that the call to 
SeqfileGetLine() will go irrevocable, so we proceed to propagate the 
irrevocability to the immediate dominators.  However, since one of the 
immediate dominators in readLoop() is the call to funky() which we've 
already processed, we die in the assert.

This assert should be an if, like the similar if a few lines up.
#else
The assert should be an if, because we can iterate multiple times 
through the same function and end up analyzing an immediate dominator 
which we already analyzed.
#endif

Had it not taken me an entire day to figure this out, I would've 
ventured to say this is an obvious fix.  Instead, I'll patiently wait 
for approval.

OK for branch?

p.s. Finally... We've fixed all the known failures in this PR.
PR 46567
	* trans-mem.c (ipa_tm_propagate_irr): Change assert to if.

Comments

Richard Henderson Feb. 11, 2011, 6:11 p.m. UTC | #1 
On 02/11/2011 06:38 AM, Aldy Hernandez wrote:
> 	PR 46567
> 	* trans-mem.c (ipa_tm_propagate_irr): Change assert to if.

Ok.


r~
diff mbox

Patch

Index: testsuite/gcc.dg/tm/pr46567-2.c
===================================================================
--- testsuite/gcc.dg/tm/pr46567-2.c	(revision 0)
+++ testsuite/gcc.dg/tm/pr46567-2.c	(revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm" } */
+
+int funky();
+int global;
+
+void SeqfileGetLine()
+{
+  funky();
+}
+
+__attribute__((transaction_callable)) void readLoop()
+{
+        SeqfileGetLine();
+        if (global)
+          funky();
+
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 170050)
+++ trans-mem.c	(working copy)
@@ -3753,8 +3753,8 @@  ipa_tm_propagate_irr (basic_block entry_
 	       son = next_dom_son (CDI_DOMINATORS, son))
 	    {
 	      /* Make sure a block isn't already in old_irr.  */
-	      gcc_assert (!old_irr || !bitmap_bit_p (old_irr, son->index));
-	      bitmap_set_bit (new_irr, son->index);
+	      if (!old_irr || !bitmap_bit_p (old_irr, son->index))
+		bitmap_set_bit (new_irr, son->index);
 	    }
 	}
     }