public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ruiyu Ni <ruiyu.ni@intel.com>
To: edk2-devel@lists.01.org
Cc: "Liming Gao" <liming.gao@intel.com>,
	"Michael Kinney" <michael.d.kinney@intel.com>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Ard Biesheuvel" <ard.biesheuvel@linaro.org>
Subject: [PATCH v2] MdePkg/BaseSynchronizationLib: Fix InternalSync[De|In]crement
Date: Wed,  7 Nov 2018 23:54:33 +0800	[thread overview]
Message-ID: <20181107155433.177700-1-ruiyu.ni@intel.com> (raw)

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>
---
 MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 4 ++--
 MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
index af39bdeb51..760a020a32 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
@@ -40,7 +40,7 @@ InternalSyncIncrement (
     "lock               \n\t"
     "xadd    %%eax, %1  \n\t"
     "inc     %%eax      \n\t"
-    : "=a" (Result),          // %0
+    : "=&a" (Result),         // %0
       "+m" (*Value)           // %1
     :                         // no inputs that aren't also outputs
     : "memory",
@@ -76,7 +76,7 @@ InternalSyncDecrement (
     "lock                \n\t"
     "xadd    %%eax, %1   \n\t"
     "dec     %%eax       \n\t"
-    : "=a" (Result),           // %0
+    : "=&a" (Result),          // %0
       "+m" (*Value)            // %1
     :                          // no inputs that aren't also outputs
     : "memory",
diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
index edb904c007..767d4626b8 100644
--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
@@ -40,7 +40,7 @@ InternalSyncIncrement (
     "lock               \n\t"
     "xadd    %%eax, %1  \n\t"
     "inc     %%eax      \n\t"
-    : "=a" (Result),          // %0
+    : "=&a" (Result),         // %0
       "+m" (*Value)           // %1
     :                         // no inputs that aren't also outputs
     : "memory",
@@ -76,7 +76,7 @@ InternalSyncDecrement (
     "lock                \n\t"
     "xadd    %%eax, %1   \n\t"
     "dec     %%eax       \n\t"
-    : "=a" (Result),           // %0
+    : "=&a" (Result),          // %0
       "+m" (*Value)            // %1
     :                          // no inputs that aren't also outputs
     : "memory",
-- 
2.16.1.windows.1



             reply	other threads:[~2018-11-07 15:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 15:54 Ruiyu Ni [this message]
2018-11-07 20:56 ` [PATCH v2] MdePkg/BaseSynchronizationLib: Fix InternalSync[De|In]crement Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181107155433.177700-1-ruiyu.ni@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox