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

Submitted by Aldy Hernandez on Jan. 12, 2011, 6:38 p.m.

Details

Message ID 4D2DF519.1020302@redhat.com
State New
Headers show

Commit Message

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.

Comments

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 hide | download patch | download mbox

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;
 }