]> git.proxmox.com Git - mirror_edk2.git/commitdiff
MdePkg/BaseSynchronizationLib: Fix InternalSync[De|In]crement
authorRuiyu Ni <ruiyu.ni@intel.com>
Wed, 7 Nov 2018 15:51:51 +0000 (23:51 +0800)
committerRuiyu Ni <ruiyu.ni@intel.com>
Thu, 8 Nov 2018 02:00:05 +0000 (10:00 +0800)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1303

Today's code generates assembly code as below for
InternalSyncIncrement:
  __asm__ __volatile__ (
    "movl    $1, %%eax  \n\t"
    "lock               \n\t"
    "xadd    %%eax, %1  \n\t"
    "inc     %%eax      \n\t"
    : "=a" (Result),          // %0
      "+m" (*Value)           // %1
    :                         // no inputs that aren't also outputs
    : "memory",
      "cc"
    );

 0:       55      pushl   %ebp
 1:       89 e5   movl    %esp, %ebp
 3:       8b 45 08        movl    8(%ebp), %eax
 6:       b8 01 00 00 00  movl    $1, %eax
 b:       f0      lock
 c:       0f c1 00        xaddl   %eax, _InternalSyncIncrement(%eax)
 f:       40      incl    %eax
10:       5d      popl    %ebp
11:       c3      retl

Line #3 and Line #6 both use EAX as destination register.
Line #c uses EAX and (EAX).

The output operand "=a" tells GCC that EAX is used for output.
But GCC only assumes that EAX will be used in the very last
instruction.

Per GCC document,
"Use the '&' constraint modifier on all output operands that must
 not  overlap an input. Otherwise, GCC may allocate the output
 operand in the same register as an unrelated input operand, on
 the assumption that the assembler code consumes its inputs before
 producing outputs. This assumption may be false if the assembler
 code actually consists of more than one instruction."

"=&a" should be used to tell GCC not use EAX before the assembly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Fixes: 8a94eb9283fa09a30f5f06f0c12cf0ee4e14fbcf
Fixes: 17634d026f968c404b039a8d8431b6389dd396ea
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c

index af39bdeb516c6ab19de6a6ebaef4627d973a37eb..760a020a327c0464ffceb1b484843ff561aab2c7 100644 (file)
@@ -40,7 +40,7 @@ InternalSyncIncrement (
     "lock               \n\t"\r
     "xadd    %%eax, %1  \n\t"\r
     "inc     %%eax      \n\t"\r
-    : "=a" (Result),          // %0\r
+    : "=&a" (Result),         // %0\r
       "+m" (*Value)           // %1\r
     :                         // no inputs that aren't also outputs\r
     : "memory",\r
@@ -76,7 +76,7 @@ InternalSyncDecrement (
     "lock                \n\t"\r
     "xadd    %%eax, %1   \n\t"\r
     "dec     %%eax       \n\t"\r
-    : "=a" (Result),           // %0\r
+    : "=&a" (Result),          // %0\r
       "+m" (*Value)            // %1\r
     :                          // no inputs that aren't also outputs\r
     : "memory",\r
index edb904c00704851f59a97f8175a328383cee31f3..767d4626b888df26672e5f2f928f611916c005ee 100644 (file)
@@ -40,7 +40,7 @@ InternalSyncIncrement (
     "lock               \n\t"\r
     "xadd    %%eax, %1  \n\t"\r
     "inc     %%eax      \n\t"\r
-    : "=a" (Result),          // %0\r
+    : "=&a" (Result),         // %0\r
       "+m" (*Value)           // %1\r
     :                         // no inputs that aren't also outputs\r
     : "memory",\r
@@ -76,7 +76,7 @@ InternalSyncDecrement (
     "lock                \n\t"\r
     "xadd    %%eax, %1   \n\t"\r
     "dec     %%eax       \n\t"\r
-    : "=a" (Result),           // %0\r
+    : "=&a" (Result),          // %0\r
       "+m" (*Value)            // %1\r
     :                          // no inputs that aren't also outputs\r
     : "memory",\r