diff mbox

[trans-mem] PR46941: Mark new/delete operators as transaction_pure

Message ID 4D49910E.8060206@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 2, 2011, 5:14 p.m. UTC 
On 01/18/11 08:44, Patrick Marlier wrote:
> I am not agree. Functions should be marked as transaction_safe not
> transaction_pure because libitm is managing those new/delete safe
> allocations.

Patrick is right.  I was wrong.  Attached are his suggestions, plus 
modifications to make it work with 32-bits.

Richard, the [new] operator which you implement in the run-time is 
mangled for 64-bits.  I added a 32-bit version when !__LP64__.  I don't 
know how thorough you'd like to check for 64/32 bits.  I'm avoiding 
having to come up with fancy autoconf checks.

OK for branch?
libitm/
	* alloc_cpp.cc: Handle non-64 bit variants for new().
gcc/
	* cp/decl.c (push_cp_library_fn): Set attribute to
	transaction_safe.
	* testsuite/g++.dg/tm/pr46941.C: Test that new/delete calls are
	transformed into calls into the runtime.

Comments

Richard Henderson Feb. 3, 2011, 4:34 p.m. UTC | #1 
On 02/02/2011 09:14 AM, Aldy Hernandez wrote:
> Richard, the [new] operator which you implement in the run-time is
> mangled for 64-bits. I added a 32-bit version when !__LP64__. I don't
> know how thorough you'd like to check for 64/32 bits. I'm avoiding
> having to come up with fancy autoconf checks.

Sadly, an autoconf check is the only way.  It's OS dependent whether
size_t is "unsigned int" or "unsigned long", even for 32-bit targets.

Thanks for catching this.

> gcc/
> 	* cp/decl.c (push_cp_library_fn): Set attribute to
> 	transaction_safe.
> 	* testsuite/g++.dg/tm/pr46941.C: Test that new/delete calls are
> 	transformed into calls into the runtime.

This part is ok.

For my sins, I will handle the autoconf-ery in the library.


r~
Aldy Hernandez Feb. 3, 2011, 4:36 p.m. UTC | #2 
> For my sins, I will handle the autoconf-ery in the library.

Yay!  Going on 11 years with nothing but a one-liner to the autoconf 
machinery!

Thanks.
Jakub Jelinek Feb. 3, 2011, 4:40 p.m. UTC | #3 
On Thu, Feb 03, 2011 at 08:34:16AM -0800, Richard Henderson wrote:
> On 02/02/2011 09:14 AM, Aldy Hernandez wrote:
> > Richard, the [new] operator which you implement in the run-time is
> > mangled for 64-bits. I added a 32-bit version when !__LP64__. I don't
> > know how thorough you'd like to check for 64/32 bits. I'm avoiding
> > having to come up with fancy autoconf checks.
> 
> Sadly, an autoconf check is the only way.  It's OS dependent whether
> size_t is "unsigned int" or "unsigned long", even for 32-bit targets.

Well, we now have __SIZE_TYPE__ predefined macro with the right type...

	Jakub
diff mbox

Patch

Index: libitm/libitm.map
===================================================================
--- libitm/libitm.map	(revision 169292)
+++ libitm/libitm.map	(working copy)
@@ -170,7 +170,9 @@  LIBITM_1.0 {
 	_ITM_dropReferences;
 
 	_ZGTtnwm;
+	_ZGTtnwj;
 	_ZGTtnam;
+	_ZGTtnaj;
 	_ZGTtdlPv;
 	_ZGTtdaPv;
 	_ZGTtnwmRKSt9nothrow_t;
Index: libitm/alloc_cpp.cc
===================================================================
--- libitm/alloc_cpp.cc	(revision 169292)
+++ libitm/alloc_cpp.cc	(working copy)
@@ -62,7 +62,11 @@  del_opvnt (void *ptr)
 
 /* Wrap: operator new (std::size_t sz)  */
 void *
+#ifdef __LP64__
 _ZGTtnwm (size_t sz)
+#else
+_ZGTtnwj (size_t sz)
+#endif
 {
   void *r = _Znwm (sz);
   if (r)
@@ -82,7 +86,11 @@  _ZGTtnwmRKSt9nothrow_t (size_t sz, c_not
 
 /* Wrap: operator new[] (std::size_t sz)  */
 void *
+#ifdef __LP64__
 _ZGTtnam (size_t sz)
+#else
+_ZGTtnaj (size_t sz)
+#endif
 {
   void *r = _Znam (sz);
   if (r)
Index: gcc/testsuite/g++.dg/tm/pr46941.C
===================================================================
--- gcc/testsuite/g++.dg/tm/pr46941.C	(revision 169292)
+++ gcc/testsuite/g++.dg/tm/pr46941.C	(working copy)
@@ -29,3 +29,9 @@  void deallocatearray(Obj *o[])
 { 
   delete [] o;
 }
+
+/* The delete/new operators are handled by the libitm runtime.  */
+/* { dg-final { scan-assembler "_ZGTtnw\[mj\]" } } */
+/* { dg-final { scan-assembler "_ZGTtna\[mj\]" } } */
+/* { dg-final { scan-assembler "_ZGTtdlPv" } } */
+/* { dg-final { scan-assembler "_ZGTtdaPv" } } */
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 169374)
+++ gcc/cp/decl.c	(working copy)
@@ -3776,7 +3776,7 @@  push_cp_library_fn (enum tree_code opera
 				 type);
   pushdecl (fn);
   if (flag_tm)
-    apply_tm_attr (fn, get_identifier ("transaction_pure"));
+    apply_tm_attr (fn, get_identifier ("transaction_safe"));
   return fn;
 }