Re: [PATCH v3] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value

["Gao, Liming" <liming.gao@intel.com>]

The patch is good. Could you submit BZ to track this change?

Thanks
Liming
>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Thursday, September 13, 2018 5:51 PM
>To: edk2-devel@lists.01.org
>Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
><liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: [PATCH v3] MdePkg/SynchronizationLib: fix
>Interlocked[De|In]crement return value
>
>Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
>perform atomic increment/decrement but doesn't guarantee the return
>value equals to the new value.
>
>The patch fixes the behavior to use "XADD" instruction to guarantee
>the return value equals to the new value.
>
>The patch calls intrinsic functions for MSVC tool chain, calls the
>NASM implementation for INTEL tool chain and calls GCC inline
>assembly implementation (GccInline.c) for GCC tool chain.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>Cc: Jiewen Yao <jiewen.yao@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>---
> MdePkg/Include/Library/SynchronizationLib.h        |  6 +--
> .../BaseSynchronizationLib.inf                     | 18 ++++-----
> .../BaseSynchronizationLibInternals.h              |  6 +--
> .../BaseSynchronizationLib/Ia32/GccInline.c        | 18 ++++-----
> .../Ia32/InterlockedDecrement.c                    | 42 ---------------------
> .../Ia32/InterlockedDecrement.nasm                 | 10 ++---
> .../Ia32/InterlockedIncrement.c                    | 43 ----------------------
> .../Ia32/InterlockedIncrement.nasm                 |  7 ++--
> ...lockedDecrement.c => InterlockedDecrementMsc.c} |  4 +-
> ...lockedIncrement.c => InterlockedIncrementMsc.c} |  4 +-
> .../BaseSynchronizationLib/Synchronization.c       |  6 +--
> .../BaseSynchronizationLib/SynchronizationGcc.c    |  6 +--
> .../BaseSynchronizationLib/SynchronizationMsc.c    |  6 +--
> .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +++++-----
> .../X64/InterlockedDecrement.nasm                  |  8 ++--
> .../X64/InterlockedIncrement.nasm                  |  5 ++-
> 16 files changed, 56 insertions(+), 152 deletions(-)
> delete mode 100644
>MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> delete mode 100644
>MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> rename
>MdePkg/Library/BaseSynchronizationLib/{X64/InterlockedDecrement.c =>
>InterlockedDecrementMsc.c} (87%)
> rename
>MdePkg/Library/BaseSynchronizationLib/{X64/InterlockedIncrement.c =>
>InterlockedIncrementMsc.c} (87%)
>
>diff --git a/MdePkg/Include/Library/SynchronizationLib.h
>b/MdePkg/Include/Library/SynchronizationLib.h
>index da69f6ff5e..ce3bce04f5 100644
>--- a/MdePkg/Include/Library/SynchronizationLib.h
>+++ b/MdePkg/Include/Library/SynchronizationLib.h
>@@ -144,8 +144,7 @@ ReleaseSpinLock (
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>@@ -166,8 +165,7 @@ InterlockedIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>index 0be1d4331f..427e959f2e 100755
>--- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>+++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>@@ -34,9 +34,9 @@ [Sources.IA32]
>   Ia32/InterlockedCompareExchange64.c | MSFT
>   Ia32/InterlockedCompareExchange32.c | MSFT
>   Ia32/InterlockedCompareExchange16.c | MSFT
>-  Ia32/InterlockedDecrement.c | MSFT
>-  Ia32/InterlockedIncrement.c | MSFT
>-  SynchronizationMsc.c  | MSFT
>+  InterlockedIncrementMsc.c | MSFT
>+  InterlockedDecrementMsc.c | MSFT
>+  SynchronizationMsc.c | MSFT
>
>   Ia32/InterlockedCompareExchange64.nasm| INTEL
>   Ia32/InterlockedCompareExchange32.nasm| INTEL
>@@ -54,17 +54,15 @@ [Sources.X64]
>   X64/InterlockedCompareExchange64.c | MSFT
>   X64/InterlockedCompareExchange32.c | MSFT
>   X64/InterlockedCompareExchange16.c | MSFT
>+  InterlockedIncrementMsc.c | MSFT
>+  InterlockedDecrementMsc.c | MSFT
>+  SynchronizationMsc.c | MSFT
>
>   X64/InterlockedCompareExchange64.nasm| INTEL
>   X64/InterlockedCompareExchange32.nasm| INTEL
>   X64/InterlockedCompareExchange16.nasm| INTEL
>-
>-  X64/InterlockedDecrement.c | MSFT
>-  X64/InterlockedIncrement.c | MSFT
>-  SynchronizationMsc.c | MSFT
>-
>-  X64/InterlockedDecrement.nasm| INTEL
>-  X64/InterlockedIncrement.nasm| INTEL
>+  X64/InterlockedDecrement.nasm | INTEL
>+  X64/InterlockedIncrement.nasm | INTEL
>   Synchronization.c | INTEL
>
>   Ia32/InternalGetSpinLockProperties.c | GCC
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
>b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
>index 37edd7c188..8c363a8585 100644
>---
>a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
>+++
>b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h
>@@ -27,8 +27,7 @@
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to increment.
>
>@@ -47,8 +46,7 @@ InternalSyncIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decrement value. The decrement operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to decrement.
>
>diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>index 4ab293f243..d82e0205f5 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>@@ -20,8 +20,7 @@
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to increment.
>
>@@ -37,9 +36,10 @@ InternalSyncIncrement (
>   UINT32  Result;
>
>   __asm__ __volatile__ (
>+    "movl    $1, %%eax  \n\t"
>     "lock               \n\t"
>-    "incl    %2         \n\t"
>-    "movl    %2, %%eax      "
>+    "xadd    %%eax, %2  \n\t"
>+    "inc     %%eax          "
>     : "=a" (Result),          // %0
>       "=m" (*Value)           // %1
>     : "m"  (*Value)           // %2
>@@ -57,8 +57,7 @@ InternalSyncIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to decrement.
>
>@@ -74,9 +73,10 @@ InternalSyncDecrement (
>    UINT32  Result;
>
>   __asm__ __volatile__ (
>-    "lock               \n\t"
>-    "decl    %2         \n\t"
>-    "movl    %2, %%eax      "
>+    "movl    $-1, %%eax  \n\t"
>+    "lock                \n\t"
>+    "xadd    %%eax, %2   \n\t"
>+    "dec     %%eax                  "
>     : "=a" (Result),          // %0
>       "=m" (*Value)           // %1
>     : "m"  (*Value)           // %2
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
>deleted file mode 100644
>index 354a0e7ab1..0000000000
>--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
>+++ /dev/null
>@@ -1,42 +0,0 @@
>-/** @file
>-  InterlockedDecrement function
>-
>-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>-  This program and the accompanying materials
>-  are licensed and made available under the terms and conditions of the BSD
>License
>-  which accompanies this distribution.  The full text of the license may be
>found at
>-  http://opensource.org/licenses/bsd-license.php.
>-
>-  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>-  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>-
>-**/
>-
>-
>-
>-
>-/**
>-  Performs an atomic decrement of an 32-bit unsigned integer.
>-
>-  Performs an atomic decrement of the 32-bit unsigned integer specified by
>-  Value and returns the decrement value. The decrement operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>-
>-  @param  Value A pointer to the 32-bit value to decrement.
>-
>-  @return The decrement value.
>-
>-**/
>-UINT32
>-EFIAPI
>-InternalSyncDecrement (
>-  IN      volatile UINT32           *Value
>-  )
>-{
>-  _asm {
>-    mov     eax, Value
>-    lock    dec     dword ptr [eax]
>-    mov     eax, [eax]
>-  }
>-}
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
>index 4c46041186..dd5a8de3ed 100644
>---
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
>+++
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
>@@ -1,6 +1,6 @@
> ;------------------------------------------------------------------------------
> ;
>-; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>+; Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> ; This program and the accompanying materials
> ; are licensed and made available under the terms and conditions of the BSD
>License
> ; which accompanies this distribution.  The full text of the license may be
>found at
>@@ -32,8 +32,8 @@
> ;------------------------------------------------------------------------------
> global ASM_PFX(InternalSyncDecrement)
> ASM_PFX(InternalSyncDecrement):
>-    mov     eax, [esp + 4]
>-    lock    dec     dword [eax]
>-    mov     eax, [eax]
>+    mov       ecx, [esp + 4]
>+    mov       eax, 0FFFFFFFFh
>+    lock xadd dword [ecx], eax
>+    dec       eax
>     ret
>-
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
>deleted file mode 100644
>index c61a550119..0000000000
>--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
>+++ /dev/null
>@@ -1,43 +0,0 @@
>-/** @file
>-  InterLockedIncrement function
>-
>-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>-  This program and the accompanying materials
>-  are licensed and made available under the terms and conditions of the BSD
>License
>-  which accompanies this distribution.  The full text of the license may be
>found at
>-  http://opensource.org/licenses/bsd-license.php.
>-
>-  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>-  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>-
>-**/
>-
>-
>-
>-
>-/**
>-  Performs an atomic increment of an 32-bit unsigned integer.
>-
>-  Performs an atomic increment of the 32-bit unsigned integer specified by
>-  Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>-
>-  @param  Value A pointer to the 32-bit value to increment.
>-
>-  @return The incremented value.
>-
>-**/
>-UINT32
>-EFIAPI
>-InternalSyncIncrement (
>-  IN      volatile UINT32           *Value
>-  )
>-{
>-  _asm {
>-    mov     eax, Value
>-    lock    inc     dword ptr [eax]
>-    mov     eax, [eax]
>-  }
>-}
>-
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
>index 3902c73275..0677e4bf78 100644
>---
>a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
>+++
>b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
>@@ -32,8 +32,9 @@
> ;------------------------------------------------------------------------------
> global ASM_PFX(InternalSyncIncrement)
> ASM_PFX(InternalSyncIncrement):
>-    mov     eax, [esp + 4]
>-    lock    inc     dword [eax]
>-    mov     eax, [eax]
>+    mov       ecx, [esp + 4]
>+    mov       eax, 1
>+    lock xadd dword [ecx], eax
>+    inc       eax
>     ret
>
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
>b/MdePkg/Library/BaseSynchronizationLib/InterlockedDecrementMsc.c
>similarity index 87%
>rename from
>MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
>rename to
>MdePkg/Library/BaseSynchronizationLib/InterlockedDecrementMsc.c
>index da402cd4c6..6dc9502ea6 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/InterlockedDecrementMsc.c
>@@ -1,7 +1,7 @@
> /** @file
>   InterlockedDecrement function
>
>-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD
>License
>   which accompanies this distribution.  The full text of the license may be
>found at
>@@ -41,6 +41,6 @@ InternalSyncDecrement (
>   IN      volatile UINT32           *Value
>   )
> {
>-  return _InterlockedDecrement ((long *)(UINTN)(Value));
>+  return _InterlockedDecrement ((long *)(Value));
> }
>
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
>b/MdePkg/Library/BaseSynchronizationLib/InterlockedIncrementMsc.c
>similarity index 87%
>rename from
>MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
>rename to
>MdePkg/Library/BaseSynchronizationLib/InterlockedIncrementMsc.c
>index bbd1384602..8327d81a87 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/InterlockedIncrementMsc.c
>@@ -1,7 +1,7 @@
> /** @file
>   InterLockedIncrement function
>
>-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD
>License
>   which accompanies this distribution.  The full text of the license may be
>found at
>@@ -41,6 +41,6 @@ InternalSyncIncrement (
>   IN      volatile UINT32           *Value
>   )
> {
>-  return _InterlockedIncrement ((long *)(UINTN)(Value));
>+  return _InterlockedIncrement ((long *)(Value));
> }
>
>diff --git a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
>b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
>index 76c5a1275c..0ab59ca702 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
>@@ -231,8 +231,7 @@ ReleaseSpinLock (
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>@@ -256,8 +255,7 @@ InterlockedIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>diff --git a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
>b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
>index 5ac548b19f..177739d3da 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
>@@ -247,8 +247,7 @@ ReleaseSpinLock (
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>@@ -272,8 +271,7 @@ InterlockedIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>diff --git a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
>b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
>index e3298c8ab4..6ab2d71870 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
>@@ -249,8 +249,7 @@ ReleaseSpinLock (
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>@@ -274,8 +273,7 @@ InterlockedIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   If Value is NULL, then ASSERT().
>
>diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
>b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
>index 5224dd063f..4c4d6e3fc7 100644
>--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
>+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
>@@ -15,14 +15,12 @@
>
>
>
>-
> /**
>   Performs an atomic increment of an 32-bit unsigned integer.
>
>   Performs an atomic increment of the 32-bit unsigned integer specified by
>   Value and returns the incremented value. The increment operation must be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to increment.
>
>@@ -38,9 +36,10 @@ InternalSyncIncrement (
>   UINT32  Result;
>
>   __asm__ __volatile__ (
>+    "movl    $1, %%eax  \n\t"
>     "lock               \n\t"
>-    "incl    %2         \n\t"
>-    "mov     %2, %%eax      "
>+    "xadd    %%eax, %2  \n\t"
>+    "inc     %%eax          "
>     : "=a" (Result),          // %0
>       "=m" (*Value)           // %1
>     : "m"  (*Value)           // %2
>@@ -57,8 +56,7 @@ InternalSyncIncrement (
>
>   Performs an atomic decrement of the 32-bit unsigned integer specified by
>   Value and returns the decremented value. The decrement operation must
>be
>-  performed using MP safe mechanisms. The state of the return value is not
>-  guaranteed to be MP safe.
>+  performed using MP safe mechanisms.
>
>   @param  Value A pointer to the 32-bit value to decrement.
>
>@@ -74,9 +72,10 @@ InternalSyncDecrement (
>    UINT32  Result;
>
>   __asm__ __volatile__ (
>-    "lock               \n\t"
>-    "decl    %2         \n\t"
>-    "mov     %2, %%eax      "
>+    "movl    $-1, %%eax  \n\t"
>+    "lock                \n\t"
>+    "xadd    %%eax, %2   \n\t"
>+    "dec     %%eax                  "
>     : "=a" (Result),          // %0
>       "=m" (*Value)           // %1
>     : "m"  (*Value)           // %2
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
>b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
>index 60f43111fe..dabdca945e 100644
>---
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
>+++
>b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
>@@ -1,6 +1,6 @@
> ;------------------------------------------------------------------------------
> ;
>-; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>+; Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> ; This program and the accompanying materials
> ; are licensed and made available under the terms and conditions of the BSD
>License
> ; which accompanies this distribution.  The full text of the license may be
>found at
>@@ -33,7 +33,7 @@
> ;------------------------------------------------------------------------------
> global ASM_PFX(InternalSyncDecrement)
> ASM_PFX(InternalSyncDecrement):
>-    lock    dec     dword [rcx]
>-    mov     eax, [rcx]
>+    mov       eax, 0FFFFFFFFh
>+    lock xadd dword [rcx], eax
>+    dec       eax
>     ret
>-
>diff --git
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
>b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
>index 7f877b5774..77379d998e 100644
>---
>a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
>+++
>b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
>@@ -33,7 +33,8 @@
> ;------------------------------------------------------------------------------
> global ASM_PFX(InternalSyncIncrement)
> ASM_PFX(InternalSyncIncrement):
>-    lock    inc     dword [rcx]
>-    mov     eax, [rcx]
>+    mov       eax, 1
>+    lock xadd dword [rcx], eax
>+    inc       eax
>     ret
>
>--
>2.16.1.windows.1