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

login
register
mail settings
Submitter Aldy Hernandez
Date Jan. 12, 2011, 6:38 p.m.
Message ID <4D2DF519.1020302@redhat.com>
Download mbox | patch
Permalink /patch/78607/
State New
Headers show

Comments

Aldy Hernandez - Jan. 12, 2011, 6:38 p.m.
Same spice, different name.

I've used push_cp_library_fn() to mark the function, as opposed to the 
caller, since only the new/delete operators use this function.

OK for branch?
* cp/decl.c (push_cp_library_fn): Mark function as
	transaction_pure.
Richard Henderson - Jan. 12, 2011, 6:59 p.m.
On 01/12/2011 10:38 AM, Aldy Hernandez wrote:
> 	* cp/decl.c (push_cp_library_fn): Mark function as
> 	transaction_pure.

Ok.


r~
Richard Guenther - Jan. 18, 2011, 3:34 p.m.
On Wed, Jan 12, 2011 at 7:59 PM, Richard Henderson <rth@redhat.com> wrote:
> On 01/12/2011 10:38 AM, Aldy Hernandez wrote:
>>       * cp/decl.c (push_cp_library_fn): Mark function as
>>       transaction_pure.
>
> Ok.

Btw, we've virorously fought adding a malloc attribute here for years
but C++ maintainers keep arguing people can arbitrarily override
this function.  So how's it now suddenly transaction-safe?

Richard.

>
> r~
>
>
Patrick Marlier - Jan. 18, 2011, 4:36 p.m.
On 01/18/2011 04:34 PM, Richard Guenther wrote:
> On Wed, Jan 12, 2011 at 7:59 PM, Richard Henderson<rth@redhat.com>  wrote:
>> On 01/12/2011 10:38 AM, Aldy Hernandez wrote:
>>>        * cp/decl.c (push_cp_library_fn): Mark function as
>>>        transaction_pure.
>>
>> Ok.
>
> Btw, we've virorously fought adding a malloc attribute here for years
> but C++ maintainers keep arguing people can arbitrarily override
> this function.  So how's it now suddenly transaction-safe?

I didn't follow this discussion about the malloc attribute but in this 
case, alternative solutions are:
* add transaction_safe attribute to libstdc++-v3/libsupc++/new
* add transaction_safe attribute to libitm/libitm.h

I think the 2nd solution is better because these transaction_safe 
new/delete operators are part of libitm but from a developer 
perspective, it is not convenient to add #include <libitm/libitm.h>.

Patrick.
Richard Henderson - Jan. 18, 2011, 4:48 p.m.
On 01/18/2011 07:34 AM, Richard Guenther wrote:
> Btw, we've virorously fought adding a malloc attribute here for years
> but C++ maintainers keep arguing people can arbitrarily override
> this function.  So how's it now suddenly transaction-safe?

Really?  I thought they were constrained to vaguely keep the
semantics of "new"...


r~
Mike Stump - Jan. 18, 2011, 7:27 p.m.
On Jan 18, 2011, at 7:34 AM, Richard Guenther wrote:
> Btw, we've virorously fought adding a malloc attribute here for years
> but C++ maintainers keep arguing people can arbitrarily override
> this function.  So how's it now suddenly transaction-safe?

I think the way this is phrased can misled people that aren't skilled in the art of language standards into misunderstanding the issue and base language standard itself.

I think a better way to express this would be to recognize that the language api standard itself gets to set the requirements for replacement functions, and in particular, the transation mem api standard gets to set the requirements of the replacement functions, and once that is done, the code change in the compiler is both trivial and correct (though, not necessarily necessary) by definition.

So, in short, this change is all about the requirements of trans-mem language standard.  The replacement functions cannot have arbitrary behavior and result in a program where the standard places _any_ requirements on behavior of that program.

Patch

Index: testsuite/g++.dg/tm/pr46941.C
===================================================================
--- testsuite/g++.dg/tm/pr46941.C	(revision 0)
+++ testsuite/g++.dg/tm/pr46941.C	(revision 0)
@@ -0,0 +1,31 @@ 
+// { dg-do "compile" }
+// { dg-options "-fgnu-tm" }
+
+class Obj
+{ 
+  int dummy;
+};
+
+__attribute__((transaction_safe))
+Obj* allocate()
+{ 
+  return new Obj;
+}
+
+__attribute__((transaction_safe))
+void deallocate(Obj * o)
+{ 
+  delete o;
+}
+
+__attribute__((transaction_safe))
+Obj* allocatearray()
+{ 
+  return new Obj[2];
+}
+
+__attribute__((transaction_safe))
+void deallocatearray(Obj *o[])
+{ 
+  delete [] o;
+}
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 168675)
+++ cp/decl.c	(working copy)
@@ -3775,6 +3775,8 @@  push_cp_library_fn (enum tree_code opera
 				 operator_code,
 				 type);
   pushdecl (fn);
+  if (flag_tm)
+    apply_tm_attr (fn, get_identifier ("transaction_pure"));
   return fn;
 }