diff mbox

[libitm] Minor changes to libitm.h

Message ID 4F1F48E1.9000507@gmail.com
State New
Headers show

Commit Message

Patrick Marlier Jan. 25, 2012, 12:12 a.m. UTC 
Hi,

I propose some minor modifications to libitm.h:
* Add a link to current TM-ABI document.
* Remove ITM_REGPARM from _ITM_beginTransaction since on x86-32, a 
variadic function ignores regparm.
* Add ITM_PURE to _ITM_addUserCommitAction and _ITM_addUserUndoAction to 
be usable inside transactions.
* Cosmetic changes to match GCC coding rules.

By the way, is it on purpose that libitm.h is not installed?

Tested on x86_64-unknown-linux-gnu. Ok for trunk?
(Aldy, I am really sorry to bother you one more time. I owe you one)

Thanks.
--
Patrick.

libitm/
2012-01-24  Patrick Marlier  <patrick.marlier@gmail.com>

         * libitm.h (_ITM_beginTransaction): Remove ITM_REGPARM.
         (_ITM_addUserCommitAction): Add ITM_PURE attribute.
         (_ITM_addUserUndoAction): Likewise.

Comments

Torvald Riegel Jan. 25, 2012, 12:32 a.m. UTC | #1 
On Tue, 2012-01-24 at 19:12 -0500, Patrick Marlier wrote:
> I propose some minor modifications to libitm.h:
> * Add a link to current TM-ABI document.

We should link to libitm.texi instead because this describes the
interface that we implement.

> * Remove ITM_REGPARM from _ITM_beginTransaction since on x86-32, a 
> variadic function ignores regparm.
> * Add ITM_PURE to _ITM_addUserCommitAction and _ITM_addUserUndoAction to 
> be usable inside transactions.

Those should be called from transaction_pure code only, or from wrapper
functions linked to using transaction_wrapper.  Thus, they don't need to
be pure.  Alternatively, why should they be?
Patrick Marlier Jan. 25, 2012, 1:36 a.m. UTC | #2 
On 01/24/2012 07:32 PM, Torvald Riegel wrote:
>> * Remove ITM_REGPARM from _ITM_beginTransaction since on x86-32, a
>> >  variadic function ignores regparm.
>> >  * Add ITM_PURE to _ITM_addUserCommitAction and _ITM_addUserUndoAction to
>> >  be usable inside transactions.
> Those should be called from transaction_pure code only, or from wrapper
> functions linked to using transaction_wrapper.  Thus, they don't need to
> be pure.  Alternatively, why should they be?

I though it was also allowed to be called inside transaction_safe code. 
This way, the developer has some callbacks on transactions events.
example:

void ucommit(void *arg)
{
   printf("Committed. now we can do undoable action (as printf) or ...\n");
}

__transaction_atomic {
   _ITM_addUserCommitAction(ucommit, _ITM_noTransactionId, NULL);
   ...
}

I read the ABI and right they seem to focus on the case of tm_wrap. In 
fact, I found those calls quite convenient sometimes but definitely not 
a problem for me.

Note that GCC does not annotate wrapped function automatically as 
transaction_pure. So the wrapper has to be transaction_pure too. I don't 
know if we should consider as a bug (Intel STM compiler adds the 
transaction_pure attribute automatically) or a feature of GCC.
In fact, I found this also quite cool to be able to propose an 
alternative and transactified function when used in transaction.

Thanks.
--
Patrick.
diff mbox

Patch

Index: libitm.h
===================================================================
--- libitm.h	(revision 183497)
+++ libitm.h	(working copy)
@@ -23,7 +23,7 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 /* The external interface of this library follows the specification described
-   in version 1 of http://www.intel.com/some/path/here.pdf.  */
+   in version 1 of http://software.intel.com/file/8097.pdf.  */
 
 #ifndef LIBITM_H
 #define LIBITM_H 1
@@ -117,7 +117,7 @@  typedef struct
     const char *psource;
 } _ITM_srcLocation;
 
-typedef void (* _ITM_userUndoFunction)(void *);
+typedef void (* _ITM_userUndoFunction) (void *);
 typedef void (* _ITM_userCommitFunction) (void *);
 
 #define _ITM_VERSION "0.90 (Feb 29 2008)"
@@ -126,28 +126,31 @@  typedef void (* _ITM_userCommitFunction) (void *);
 extern int _ITM_versionCompatible (int) ITM_REGPARM;
 extern const char * _ITM_libraryVersion (void) ITM_REGPARM;
 
-void _ITM_error(const _ITM_srcLocation *, int errorCode)
+void _ITM_error (const _ITM_srcLocation *, int errorCode)
   ITM_REGPARM ITM_NORETURN;
 
-extern _ITM_howExecuting _ITM_inTransaction(void) ITM_REGPARM;
+extern _ITM_howExecuting _ITM_inTransaction (void) ITM_REGPARM;
 
 typedef uint64_t _ITM_transactionId_t;	/* Transaction identifier */
 #define _ITM_noTransactionId 1		/* Id for non-transactional code. */
 
-extern _ITM_transactionId_t _ITM_getTransactionId(void) ITM_REGPARM;
+extern _ITM_transactionId_t _ITM_getTransactionId (void) ITM_REGPARM;
 
-extern uint32_t _ITM_beginTransaction(uint32_t, ...) ITM_REGPARM;
+/* ITM_REGPARM is ignored with variadic function on x86-32. */
+extern uint32_t _ITM_beginTransaction (uint32_t, ...);
 
-extern void _ITM_abortTransaction(_ITM_abortReason) ITM_REGPARM ITM_NORETURN;
+extern void _ITM_abortTransaction (_ITM_abortReason) ITM_REGPARM ITM_NORETURN;
 
 extern void _ITM_commitTransaction (void) ITM_REGPARM;
 
 extern void _ITM_changeTransactionMode (_ITM_transactionState) ITM_REGPARM;
 
-extern void _ITM_addUserCommitAction(_ITM_userCommitFunction,
-				     _ITM_transactionId_t, void *) ITM_REGPARM;
+extern void _ITM_addUserCommitAction (_ITM_userCommitFunction,
+				      _ITM_transactionId_t, void *)
+		ITM_REGPARM ITM_PURE;
 
-extern void _ITM_addUserUndoAction(_ITM_userUndoFunction, void *) ITM_REGPARM;
+extern void _ITM_addUserUndoAction (_ITM_userUndoFunction, void *)
+		ITM_REGPARM ITM_PURE;
 
 extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE;
 
@@ -230,41 +233,41 @@  ITM_LOG(CE)
 
 extern void _ITM_LB (const void *, size_t) ITM_REGPARM;
 
-extern void _ITM_memcpyRnWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRnWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRnWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaRWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaRWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaRWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaRWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaWWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaWWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaWWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memcpyRtaWWtaW(void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRnWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRnWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRnWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaRWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaRWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaRWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaRWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaWWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaWWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaWWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memcpyRtaWWtaW (void *, const void *, size_t) ITM_REGPARM;
 
-extern void _ITM_memmoveRnWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRnWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRnWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaRWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaRWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaRWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaRWtaW(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaWWn(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaWWt(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaWWtaR(void *, const void *, size_t) ITM_REGPARM;
-extern void _ITM_memmoveRtaWWtaW(void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRnWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRnWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRnWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaRWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaRWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaRWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaRWtaW (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaWWn (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaWWt (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaWWtaR (void *, const void *, size_t) ITM_REGPARM;
+extern void _ITM_memmoveRtaWWtaW (void *, const void *, size_t) ITM_REGPARM;
 
-extern void _ITM_memsetW(void *, int, size_t) ITM_REGPARM;
-extern void _ITM_memsetWaR(void *, int, size_t) ITM_REGPARM;
-extern void _ITM_memsetWaW(void *, int, size_t) ITM_REGPARM;
+extern void _ITM_memsetW (void *, int, size_t) ITM_REGPARM;
+extern void _ITM_memsetWaR (void *, int, size_t) ITM_REGPARM;
+extern void _ITM_memsetWaW (void *, int, size_t) ITM_REGPARM;
 
 // ??? These are not yet in the official spec; still work-in-progress.
 
@@ -277,7 +280,7 @@  extern void *_ITM_cxa_allocate_exception (size_t);
 extern void _ITM_cxa_throw (void *obj, void *tinfo, void *dest);
 extern void *_ITM_cxa_begin_catch (void *exc_ptr);
 extern void _ITM_cxa_end_catch (void);
-extern void _ITM_commitTransactionEH(void *exc_ptr) ITM_REGPARM;
+extern void _ITM_commitTransactionEH (void *exc_ptr) ITM_REGPARM;
 
 #ifdef __cplusplus
 } /* extern "C" */