Re: [ros-dev] [ros-diffs] [dgorbachev] 26482: Use allocation function instead of hack, save 12 bytes.

[Thomas Weidenmueller]

This change is *wrong*. It won't guarantee the correct alignment!!!

- Thomas

dgorbachev(a)svn.reactos.org wrote:
>   Author: dgorbachev
>  Date: Tue Apr 24 02:06:34 2007
>  New Revision: 26482
>  
>  URL: http://svn.reactos.org/svn/reactos?rev=26482&view=rev
>  Log:
>  Use allocation function instead of hack, save 12 bytes.
>  
>  Modified:
>      trunk/reactos/ntoskrnl/include/internal/ex.h
>  
>  Modified: trunk/reactos/ntoskrnl/include/internal/ex.h
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
>  ==============================================================================
>  --- trunk/reactos/ntoskrnl/include/internal/ex.h (original)
>  +++ trunk/reactos/ntoskrnl/include/internal/ex.h Tue Apr 24 02:06:34 2007
>  @@ -102,22 +102,13 @@
>   #define MAX_HIGH_INDEX      (MID_LEVEL_ENTRIES * MID_LEVEL_ENTRIES * LOW_LEVEL_ENTRIES)
>   
>   //
>  -// Detect GCC 4.1.2+
>  +// Detect GCC
>   //
>  -#if 1 // (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) < 40102
>  -
>  -//
>  -// Broken GCC with Alignment Bug. We'll do alignment ourselves at higher cost.
>  -//
>  -#define DEFINE_WAIT_BLOCK(x)                                \
>  -    struct _AlignHack                                       \
>  -    {                                                       \
>  -        UCHAR Hack[15];                                     \
>  -        EX_PUSH_LOCK_WAIT_BLOCK UnalignedBlock;             \
>  -    } WaitBlockBuffer;                                      \
>  -    PEX_PUSH_LOCK_WAIT_BLOCK x = (PEX_PUSH_LOCK_WAIT_BLOCK) \
>  -        ((ULONG_PTR)&WaitBlockBuffer.UnalignedBlock &~ 0xF);
>  -        
>  +#ifdef __GNUC__
>  +
>  +#define DEFINE_WAIT_BLOCK(x)                                                        \
>  +    PEX_PUSH_LOCK_WAIT_BLOCK x = __builtin_alloca(sizeof(EX_PUSH_LOCK_WAIT_BLOCK));
>  +    
>   #else
>   
>   //
>  @@ -125,8 +116,8 @@
>   // local variable (the actual pointer) away, so we don't take any perf hit
>   // by doing this.
>   //
>  -#define DEFINE_WAIT_BLOCK(x)                                \
>  -    EX_PUSH_LOCK_WAIT_BLOCK WaitBlockBuffer;                \
>  +#define DEFINE_WAIT_BLOCK(x)                                                        \
>  +    EX_PUSH_LOCK_WAIT_BLOCK WaitBlockBuffer;                                        \
>       PEX_PUSH_LOCK_WAIT_BLOCK x = &WaitBlockBuffer;
>       
>   #endif
>  @@ -725,7 +716,7 @@
>       if (InterlockedBitTestAndSet((PLONG)PushLock, EX_PUSH_LOCK_LOCK_V))
>       {
>           /* Someone changed it, use the slow path */
>  -        DbgPrint("%s - Contention!\n", __FUNCTION__);
>  +        // DbgPrint("%s - Contention!\n", __FUNCTION__);
>           ExfAcquirePushLockExclusive(PushLock);
>       }
>   
>  @@ -763,7 +754,7 @@
>       if (ExpChangePushlock(PushLock, NewValue.Ptr, 0))
>       {
>           /* Someone changed it, use the slow path */
>  -        DbgPrint("%s - Contention!\n", __FUNCTION__);
>  +        // DbgPrint("%s - Contention!\n", __FUNCTION__);
>           ExfAcquirePushLockShared(PushLock);
>       }
>   
>  @@ -977,7 +968,7 @@
>            OldValue.Ptr))
>       {
>           /* We have waiters, use the long path */
>  -        DbgPrint("%s - Contention!\n", __FUNCTION__);
>  +        // DbgPrint("%s - Contention!\n", __FUNCTION__);
>           ExfReleasePushLock(PushLock);
>       }
>   }
>  
>  
>

Hi,

>   This change is *wrong*. It won't guarantee the
> correct alignment!!! 
@ExfAcquirePushLockShared@4():
ntoskrnl/ex/pushlock.c:611
     push   %ebp
     mov    %esp,%ebp
     push   %edi
     push   %esi
     push   %ebx
     sub    $0x4c,%esp
     mov    %ecx,0xffffffec(%ebp)
ntoskrnl/ex/pushlock.c:612
     mov    (%ecx),%edi
ntoskrnl/ex/pushlock.c:615
     lea    0xf(%esp),%esi
->  and    $0xfffffff0,%esi
ntoskrnl/ex/pushlock.c:621
     ...

[Alex Ionescu]

You are just getting lucky. Thomas is right, builtin_alloca does NOT
guarantee alignment.

-----Original Message-----
From: ros-dev-bounces(a)reactos.org [mailto:[e-mail]] On
Behalf Of hto(a)mail.cnt.ru
Sent: April-24-07 12:19 AM
To: ReactOS Development List
Subject: Re: [ros-dev] [ros-diffs] [dgorbachev] 26482: Use allocation
function instead of hack, save 12 bytes.

Hi,

>   This change is *wrong*. It won't guarantee the
> correct alignment!!! 
@ExfAcquirePushLockShared@4():
ntoskrnl/ex/pushlock.c:611
     push   %ebp
     mov    %esp,%ebp
     push   %edi
     push   %esi
     push   %ebx
     sub    $0x4c,%esp
     mov    %ecx,0xffffffec(%ebp)
ntoskrnl/ex/pushlock.c:612
     mov    (%ecx),%edi
ntoskrnl/ex/pushlock.c:615
     lea    0xf(%esp),%esi
->  and    $0xfffffff0,%esi
ntoskrnl/ex/pushlock.c:621
     ...
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

OK, I will revert it.