diff mbox

[libitm] Remove variadic argument of _ITM_beginTransaction from libitm.h

Message ID 4EFBCA88.7060801@gmail.com
State New
Headers show

Commit Message

Patrick Marlier Dec. 29, 2011, 2:03 a.m. UTC 
With i386, the regparm(2) is not taken into account when there is a 
variadic function. All parameters are in the stack.
Since this variable argument is never used removing it is not a problem.

This solves libitm testcases memset-1.c/memcpy-1.c on i686 (part of 
PR51655/51124).

Before:
FAIL: libitm.c/memcpy-1.c execution test
FAIL: libitm.c/memset-1.c execution test

                 === libitm Summary ===

# of expected passes            21
# of unexpected failures        2
# of expected failures          5
# of unresolved testcases       1


After:
                 === libitm Summary ===

# of expected passes            23
# of expected failures          5
# of unresolved testcases       1

Tested on i686. If ok, please commit. Thanks.

Patrick Marlier.


2011-12-28  Patrick Marlier  <patrick.marlier@gmail.com>

         PR testsuite/51655
         * libitm.h (_ITM_beginTransaction): Remove unused argument.

Comments

Torvald Riegel Dec. 29, 2011, 11:59 a.m. UTC | #1 
On Wed, 2011-12-28 at 21:03 -0500, Patrick Marlier wrote:
> With i386, the regparm(2) is not taken into account when there is a 
> variadic function. All parameters are in the stack.
> Since this variable argument is never used removing it is not a problem.

The ABI specifies beginTransaction as a variadic function, however.
Even though we don't use that currently, there were reasons why we added
that in the first place (e.g., to be able to add additional bits later
on, including support for more codepaths).

Is there another way we can fix this?  Or is regparm in general
undefined for variadic functions?
Patrick Marlier Dec. 29, 2011, 4:52 p.m. UTC | #2 
On 12/29/2011 06:59 AM, Torvald Riegel wrote:
> On Wed, 2011-12-28 at 21:03 -0500, Patrick Marlier wrote:
>> With i386, the regparm(2) is not taken into account when there is a
>> variadic function. All parameters are in the stack.
>> Since this variable argument is never used removing it is not a problem.
>
> The ABI specifies beginTransaction as a variadic function, however.
> Even though we don't use that currently, there were reasons why we added
> that in the first place (e.g., to be able to add additional bits later
> on, including support for more codepaths).
I am pretty sure this is a bug in the ABI. AFAIU in 32bits, a function 
cannot be variadic and satisfy regparm(2).

> Is there another way we can fix this?  Or is regparm in general
> undefined for variadic functions?

in i386.c (init_cumulative_args):
   ...
   if (!TARGET_64BIT)
     {
       /* If there are variable arguments, then we won't pass anything
          in registers in 32-bit mode. */
       if (stdarg_p (fntype))
         {
           cum->nregs = 0;
           cum->sse_nregs = 0;
           cum->mmx_nregs = 0;
           cum->warn_avx = 0;
           cum->warn_sse = 0;
           cum->warn_mmx = 0;
           return;
         }

So I don't really see another way to fix it.
Maybe Master Aldy or Master Richard?

Patrick.
diff mbox

Patch

Index: libitm.h
===================================================================
--- libitm.h    (revision 182549)
+++ libitm.h    (working copy)
@@ -136,7 +136,7 @@  typedef uint64_t _ITM_transactionId_t;      /* Transact

  extern _ITM_transactionId_t _ITM_getTransactionId(void) ITM_REGPARM;

-extern uint32_t _ITM_beginTransaction(uint32_t, ...) ITM_REGPARM;
+extern uint32_t _ITM_beginTransaction(uint32_t) ITM_REGPARM;

  extern void _ITM_abortTransaction(_ITM_abortReason) ITM_REGPARM 
ITM_NORETURN;