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

[e-mail] 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/e...
> ==============================================================================
> --- 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: [e-mail] [mailto:[e-mail]] On
Behalf Of [e-mail]
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
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev

OK, I will revert it.