From patchwork Mon Feb 14 18:03:36 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [trans-mem] Add missing argument for _ITM_changeTransactionMode
From: Aldy Hernandez <aldyh@redhat.com>
X-Patchwork-Id: 83144
Message-Id: <4D596E78.5040600@redhat.com>
To: Patrick Marlier <patrick.marlier@unine.ch>
Cc: gcc-patches@gcc.gnu.org, rth@redhat.com, javier.arias@bsc.es
Date: Mon, 14 Feb 2011 12:03:36 -0600

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.


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)