Patchwork [trans-mem] Add missing argument for _ITM_changeTransactionMode

login
register
mail settings
Submitter Aldy Hernandez
Date Feb. 14, 2011, 6:03 p.m.
Message ID <4D596E78.5040600@redhat.com>
Download mbox | patch
Permalink /patch/83144/
State New
Headers show

Comments

Aldy Hernandez - Feb. 14, 2011, 6:03 p.m.
On 02/11/11 12:40, Patrick Marlier wrote:
> The argument for _ITM_changeTransactionMode was missing. The patch adds
> it and also correct the value of MODE_SERIALIRREVOCABLE.
>
> Patrick Marlier.

Hi Patrick.

This is an obvious patch, and I will commit it as such.

On your way to becoming a contributor, you must also test the patch, and 
add a suitable testcase.  Testing showed that there were a few other 
places that needed to be changed, once we changed the arguments to 
_ITM_changeTransactionMode.  See attached patch.

Be that as it may, it's not polite to look a gift horse in the mouth, so 
thank you so much for finding this, and for providing a patch.

BTW, if you only make changes to the trans-mem.[ch], and you're lazy, 
you can get away with only testing the TM gcc infrastructure with:

	make check-gcc RUNTESTFLAGS=tm.exp

and the runtime with:

	make check-target-libitm

However, testing just tm.exp will miss any possible warnings that are 
treated as errors on a full bootstrap (stage2 and above).  So it's 
always good to do a full bootstrap and make check.

Thanks again.
Aldy
* trans-mem.c (MODE_SERIALIRREVOCABLE): Change to 0.
	(ipa_tm_insert_irr_call): Add missing argument for
	_ITM_changeTransactionMode.

Patch

Index: testsuite/gcc.dg/tm/irrevocable-7.c
===================================================================
--- testsuite/gcc.dg/tm/irrevocable-7.c	(revision 0)
+++ testsuite/gcc.dg/tm/irrevocable-7.c	(revision 0)
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+extern void bark(void);
+
+__attribute__((transaction_callable))
+int foo()
+{
+      bark();
+}
+
+/* { dg-final { scan-ipa-dump-times "changeTransactionMode \\(0\\)" 1 "tmipa" } } */
+/* { dg-final { cleanup-ipa-dump "tmipa" } } */
Index: testsuite/gcc.dg/tm/irrevocable-2.c
===================================================================
--- testsuite/gcc.dg/tm/irrevocable-2.c	(revision 170050)
+++ testsuite/gcc.dg/tm/irrevocable-2.c	(working copy)
@@ -11,7 +11,7 @@  foo()
 {
 	__transaction [[relaxed]] {
 		global++;
-		__builtin__ITM_changeTransactionMode ();
+		__builtin__ITM_changeTransactionMode (0);
 		george++;
 	}
 }
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 170057)
+++ trans-mem.c	(working copy)
@@ -55,7 +55,7 @@ 
 #define AR_EXCEPTIONBLOCKABORT	0x0008
 #define AR_OUTERABORT		0x0010
 
-#define MODE_SERIALIRREVOCABLE	0x0001
+#define MODE_SERIALIRREVOCABLE	0x0000
 
 
 /* The representation of a transaction changes several times during the
@@ -4186,7 +4186,8 @@  ipa_tm_insert_irr_call (struct cgraph_no
 
   transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE);
 
-  g = gimple_build_call (built_in_decls[BUILT_IN_TM_IRREVOCABLE], 0);
+  g = gimple_build_call (built_in_decls[BUILT_IN_TM_IRREVOCABLE],
+                         1, build_int_cst (NULL_TREE, MODE_SERIALIRREVOCABLE));
 
   split_block_after_labels (bb);
   gsi = gsi_after_labels (bb);
Index: gtm-builtins.def
===================================================================
--- gtm-builtins.def	(revision 170050)
+++ gtm-builtins.def	(working copy)
@@ -8,7 +8,7 @@  DEF_TM_BUILTIN (BUILT_IN_TM_COMMIT_EH, "
 DEF_TM_BUILTIN (BUILT_IN_TM_ABORT, "_ITM_abortTransaction",
 		BT_FN_INT, ATTR_TM_NORETURN_NOTHROW_LIST)
 DEF_TM_BUILTIN (BUILT_IN_TM_IRREVOCABLE, "_ITM_changeTransactionMode",
-		BT_FN_INT, ATTR_TM_NOTHROW_LIST)
+		BT_FN_INT_INT, ATTR_TM_NOTHROW_LIST)
 
 DEF_TM_BUILTIN (BUILT_IN_TM_MEMCPY, "_ITM_memcpyRtWt",
 		BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_TM_TMPURE_NOTHROW_LIST)