Patchwork [trans-mem] issue with openmp

login
register
mail settings
Submitter Aldy Hernandez
Date July 6, 2010, 6:12 p.m.
Message ID <20100706181221.GA21220@redhat.com>
Download mbox | patch
Permalink /patch/58052/
State New
Headers show

Comments

Aldy Hernandez - July 6, 2010, 6:12 p.m.
On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote:
> On 07/06/2010 09:29 AM, Aldy Hernandez wrote:
> > -__attribute__((transaction_pure))
> > -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM;
> > +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE;
> > +
> > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
> > +extern void *_ITM_malloc (size_t);
> > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
> > +extern void *_ITM_calloc (size_t, size_t);
> > +extern  void _ITM_free (void *) ITM_REGPARM ITM_PURE;
> 
> I'm not sure that _ITM_malloc et al should include ITM_REGPARM.

Arghh, I was getting confused by a complaint by Patrick about a
%eax<->%rax problem in the calling sequence, but that must be something
Richard Henderson - July 6, 2010, 7:28 p.m.
On 07/06/2010 11:12 AM, Aldy Hernandez wrote:
> On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote:
>> On 07/06/2010 09:29 AM, Aldy Hernandez wrote:
>>> -__attribute__((transaction_pure))
>>> -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM;
>>> +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE;
>>> +
>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
>>> +extern void *_ITM_malloc (size_t);
>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
>>> +extern void *_ITM_calloc (size_t, size_t);
>>> +extern  void _ITM_free (void *) ITM_REGPARM ITM_PURE;
>>
>> I'm not sure that _ITM_malloc et al should include ITM_REGPARM.
> 
> Arghh, I was getting confused by a complaint by Patrick about a
> %eax<->%rax problem in the calling sequence, but that must be something
> different because I can't reproduce it.

That was due to C's implicit type of "int" for undeclared functions.

> 	* libitm.h (ITM_PURE): Define.
> 	Declare _ITM_malloc, _ITM_calloc, and _ITM_free.

Ok.


r~
Patrick Marlier - July 6, 2010, 8:58 p.m.
On 07/06/2010 09:28 PM, Richard Henderson wrote:
> On 07/06/2010 11:12 AM, Aldy Hernandez wrote:
>> On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote:
>>> On 07/06/2010 09:29 AM, Aldy Hernandez wrote:
>>>> -__attribute__((transaction_pure))
>>>> -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM;
>>>> +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE;
>>>> +
>>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
>>>> +extern void *_ITM_malloc (size_t);
>>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE
>>>> +extern void *_ITM_calloc (size_t, size_t);
>>>> +extern  void _ITM_free (void *) ITM_REGPARM ITM_PURE;
>>>
>>> I'm not sure that _ITM_malloc et al should include ITM_REGPARM.

Can you explain me why _ITM_malloc shouldn't have ITM_REGPARM whereas 
for example _ITM_memsetW must have it?

>> Arghh, I was getting confused by a complaint by Patrick about a
>> %eax<->%rax problem in the calling sequence, but that must be something
>> different because I can't reproduce it.
>
> That was due to C's implicit type of "int" for undeclared functions.
>
>> 	* libitm.h (ITM_PURE): Define.
>> 	Declare _ITM_malloc, _ITM_calloc, and _ITM_free.

Yes it solved my issue.

Thank you!

Patrick.
Richard Henderson - July 6, 2010, 11:27 p.m.
On 07/06/2010 01:58 PM, Patrick Marlier wrote:
> Can you explain me why _ITM_malloc shouldn't have ITM_REGPARM whereas
> for example _ITM_memsetW must have it?

The other functions in that header are defined as part of the Intel ABI,
and that ABI specifies REGPARM.  _ITM_malloc is not part of that ABI.


r~

Patch

different because I can't reproduce it.  Direct calls to _ITM_malloc()
agree with the calling convention expected by such function.

Patrick, if after a complete toolchain rebuild with this patch, you
still see an ABI problem, send me the testcase.

> Also, it's canonical to place these attributes before the ";",
> not before the "extern".

Fixed.

OK?

	* libitm.h (ITM_PURE): Define.
	Declare _ITM_malloc, _ITM_calloc, and _ITM_free.

Index: libitm.h
===================================================================
--- libitm.h	(revision 161512)
+++ libitm.h	(working copy)
@@ -43,6 +43,7 @@  extern "C" {
 #endif
 
 #define ITM_NORETURN	__attribute__((noreturn))
+#define ITM_PURE __attribute__((transaction_pure))
 
 /* The following are externally visible definitions and functions, though
    only very few of these should be called by user code.  */
@@ -152,8 +153,15 @@  extern void _ITM_addUserUndoAction(_ITM_
 
 extern int _ITM_getThreadnum(void) ITM_REGPARM;
 
-__attribute__((transaction_pure))
-extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM;
+extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE;
+
+extern void *_ITM_malloc (size_t)
+       __attribute__((__malloc__)) ITM_PURE;
+
+extern void *_ITM_calloc (size_t, size_t)
+       __attribute__((__malloc__)) ITM_PURE;
+
+extern  void _ITM_free (void *) ITM_PURE;
 
 
 /* The following typedefs exist to make the macro expansions below work