* [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
@ 2018-09-29 22:23 Laszlo Ersek
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
Repo: https://github.com/lersek/edk2.git
Branch: inline_asm_rw_ops_1208
This series mainly fixes the operand constraints (missing input-output
qualifications) in "BaseSynchronizationLib/*/GccInline.c".
(It would be better to remove these files altogether in favor of the
already existing NASM implementation, but due to
<https://bugzilla.tianocore.org/show_bug.cgi?id=881>, we can't generally
do that in libraries yet.)
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Thanks,
Laszlo
Laszlo Ersek (5):
MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
MdePkg/BaseSynchronizationLib GCC: fix X64
InternalSyncCompareExchange64()
MdePkg/BaseSynchronizationLib GCC: simplify IA32
InternalSyncCompareExchange64()
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 42 +++++++----------
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 47 +++++++-------------
2 files changed, 34 insertions(+), 55 deletions(-)
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
@ 2018-09-29 22:23 ` Laszlo Ersek
2018-10-01 10:17 ` Philippe Mathieu-Daudé
2018-10-18 2:00 ` Ni, Ruiyu
2018-09-29 22:23 ` [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16() Laszlo Ersek
` (5 subsequent siblings)
6 siblings, 2 replies; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
The "GccInline.c" files have some inconsistent whitespace, and missing (or
incorrect) operand comments. Fix and unify them.
This patch doesn't change behavior.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 35 ++++++-------
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 53 +++++++++-----------
2 files changed, 39 insertions(+), 49 deletions(-)
diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
index fa2be7f4b35c..1976720ac636 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
@@ -39,7 +39,7 @@ InternalSyncIncrement (
"movl $1, %%eax \n\t"
"lock \n\t"
"xadd %%eax, %1 \n\t"
- "inc %%eax "
+ "inc %%eax \n\t"
: "=a" (Result), // %0
"+m" (*Value) // %1
: // no inputs that aren't also outputs
@@ -48,7 +48,6 @@ InternalSyncIncrement (
);
return Result;
-
}
@@ -76,10 +75,10 @@ InternalSyncDecrement (
"movl $-1, %%eax \n\t"
"lock \n\t"
"xadd %%eax, %1 \n\t"
- "dec %%eax "
- : "=a" (Result), // %0
- "+m" (*Value) // %1
- : // no inputs that aren't also outputs
+ "dec %%eax \n\t"
+ : "=a" (Result), // %0
+ "+m" (*Value) // %1
+ : // no inputs that aren't also outputs
: "memory",
"cc"
);
@@ -87,6 +86,7 @@ InternalSyncDecrement (
return Result;
}
+
/**
Performs an atomic compare exchange operation on a 16-bit unsigned integer.
@@ -113,15 +113,13 @@ InternalSyncCompareExchange16 (
IN UINT16 ExchangeValue
)
{
-
__asm__ __volatile__ (
- " \n\t"
"lock \n\t"
"cmpxchgw %1, %2 \n\t"
- : "=a" (CompareValue)
- : "q" (ExchangeValue),
- "m" (*Value),
- "0" (CompareValue)
+ : "=a" (CompareValue) // %0
+ : "q" (ExchangeValue), // %1
+ "m" (*Value), // %2
+ "0" (CompareValue) // %3
: "memory",
"cc"
);
@@ -129,6 +127,7 @@ InternalSyncCompareExchange16 (
return CompareValue;
}
+
/**
Performs an atomic compare exchange operation on a 32-bit unsigned integer.
@@ -155,15 +154,13 @@ InternalSyncCompareExchange32 (
IN UINT32 ExchangeValue
)
{
-
__asm__ __volatile__ (
- " \n\t"
"lock \n\t"
"cmpxchgl %1, %2 \n\t"
- : "=a" (CompareValue) // %0
- : "q" (ExchangeValue), // %1
- "m" (*Value), // %2
- "0" (CompareValue) // %4
+ : "=a" (CompareValue) // %0
+ : "q" (ExchangeValue), // %1
+ "m" (*Value), // %2
+ "0" (CompareValue) // %3
: "memory",
"cc"
);
@@ -171,6 +168,7 @@ InternalSyncCompareExchange32 (
return CompareValue;
}
+
/**
Performs an atomic compare exchange operation on a 64-bit unsigned integer.
@@ -197,7 +195,6 @@ InternalSyncCompareExchange64 (
)
{
__asm__ __volatile__ (
- " \n\t"
"push %%ebx \n\t"
"movl %2,%%ebx \n\t"
"lock \n\t"
diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
index ab7efe23c4db..0212798d7a27 100644
--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
@@ -39,7 +39,7 @@ InternalSyncIncrement (
"movl $1, %%eax \n\t"
"lock \n\t"
"xadd %%eax, %1 \n\t"
- "inc %%eax "
+ "inc %%eax \n\t"
: "=a" (Result), // %0
"+m" (*Value) // %1
: // no inputs that aren't also outputs
@@ -75,10 +75,10 @@ InternalSyncDecrement (
"movl $-1, %%eax \n\t"
"lock \n\t"
"xadd %%eax, %1 \n\t"
- "dec %%eax "
- : "=a" (Result), // %0
- "+m" (*Value) // %1
- : // no inputs that aren't also outputs
+ "dec %%eax \n\t"
+ : "=a" (Result), // %0
+ "+m" (*Value) // %1
+ : // no inputs that aren't also outputs
: "memory",
"cc"
);
@@ -113,16 +113,14 @@ InternalSyncCompareExchange16 (
IN UINT16 ExchangeValue
)
{
-
-
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgw %3, %1 "
- : "=a" (CompareValue),
- "=m" (*Value)
- : "a" (CompareValue),
- "r" (ExchangeValue),
- "m" (*Value)
+ "cmpxchgw %3, %1 \n\t"
+ : "=a" (CompareValue), // %0
+ "=m" (*Value) // %1
+ : "a" (CompareValue), // %2
+ "r" (ExchangeValue), // %3
+ "m" (*Value) // %4
: "memory",
"cc"
);
@@ -157,16 +155,14 @@ InternalSyncCompareExchange32 (
IN UINT32 ExchangeValue
)
{
-
-
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgl %3, %1 "
- : "=a" (CompareValue), // %0
- "=m" (*Value) // %1
- : "a" (CompareValue), // %2
- "r" (ExchangeValue), // %3
- "m" (*Value)
+ "cmpxchgl %3, %1 \n\t"
+ : "=a" (CompareValue), // %0
+ "=m" (*Value) // %1
+ : "a" (CompareValue), // %2
+ "r" (ExchangeValue), // %3
+ "m" (*Value) // %4
: "memory",
"cc"
);
@@ -200,20 +196,17 @@ InternalSyncCompareExchange64 (
IN UINT64 ExchangeValue
)
{
-
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgq %3, %1 "
- : "=a" (CompareValue), // %0
- "=m" (*Value) // %1
- : "a" (CompareValue), // %2
- "r" (ExchangeValue), // %3
- "m" (*Value)
+ "cmpxchgq %3, %1 \n\t"
+ : "=a" (CompareValue), // %0
+ "=m" (*Value) // %1
+ : "a" (CompareValue), // %2
+ "r" (ExchangeValue), // %3
+ "m" (*Value) // %4
: "memory",
"cc"
);
return CompareValue;
}
-
-
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
@ 2018-09-29 22:23 ` Laszlo Ersek
2018-10-01 10:26 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32() Laszlo Ersek
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
The CMPXCHG instruction has the following operands:
- AX (implicit, CompareValue): input and output
- destination operand (*Value): input and output
- source operand (ExchangeValue): input
The IA32 version of InternalSyncCompareExchange16() correctly marks
CompareValue as input/output, but it marks (*Value) only as input.
The X64 version of InternalSyncCompareExchange16() attempts to mark both
CompareValue and (*Value) as input/output, but it doesn't use the
appropriate constraints for either operand.
Fix these issues. Furthermore, prefer the short "+" constraint for I/O
operands over the <output-operand-number> constraint that can be applied
to the input instances of I/O operands.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 9 ++++-----
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
index 1976720ac636..bd98a5aebfe7 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
@@ -115,11 +115,10 @@ InternalSyncCompareExchange16 (
{
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgw %1, %2 \n\t"
- : "=a" (CompareValue) // %0
- : "q" (ExchangeValue), // %1
- "m" (*Value), // %2
- "0" (CompareValue) // %3
+ "cmpxchgw %2, %1 \n\t"
+ : "+a" (CompareValue), // %0
+ "+m" (*Value) // %1
+ : "q" (ExchangeValue) // %2
: "memory",
"cc"
);
diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
index 0212798d7a27..4338fb8e65e8 100644
--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
@@ -115,12 +115,10 @@ InternalSyncCompareExchange16 (
{
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgw %3, %1 \n\t"
- : "=a" (CompareValue), // %0
- "=m" (*Value) // %1
- : "a" (CompareValue), // %2
- "r" (ExchangeValue), // %3
- "m" (*Value) // %4
+ "cmpxchgw %2, %1 \n\t"
+ : "+a" (CompareValue), // %0
+ "+m" (*Value) // %1
+ : "r" (ExchangeValue) // %2
: "memory",
"cc"
);
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
2018-09-29 22:23 ` [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16() Laszlo Ersek
@ 2018-09-29 22:23 ` Laszlo Ersek
2018-10-01 10:26 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64() Laszlo Ersek
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
(This patch is identical to the last one, except for the
InternalSyncCompareExchange16() -> InternalSyncCompareExchange32() and
"cmpxchgw" -> "cmpxchgl" replacements.)
The CMPXCHG instruction has the following operands:
- AX (implicit, CompareValue): input and output
- destination operand (*Value): input and output
- source operand (ExchangeValue): input
The IA32 version of InternalSyncCompareExchange32() correctly marks
CompareValue as input/output, but it marks (*Value) only as input.
The X64 version of InternalSyncCompareExchange32() attempts to mark both
CompareValue and (*Value) as input/output, but it doesn't use the
appropriate constraints for either operand.
Fix these issues. Furthermore, prefer the short "+" constraint for I/O
operands over the <output-operand-number> constraint that can be applied
to the input instances of I/O operands.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 9 ++++-----
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
index bd98a5aebfe7..44188e265af2 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
@@ -155,11 +155,10 @@ InternalSyncCompareExchange32 (
{
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgl %1, %2 \n\t"
- : "=a" (CompareValue) // %0
- : "q" (ExchangeValue), // %1
- "m" (*Value), // %2
- "0" (CompareValue) // %3
+ "cmpxchgl %2, %1 \n\t"
+ : "+a" (CompareValue), // %0
+ "+m" (*Value) // %1
+ : "q" (ExchangeValue) // %2
: "memory",
"cc"
);
diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
index 4338fb8e65e8..a85cf0265c8b 100644
--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
@@ -155,12 +155,10 @@ InternalSyncCompareExchange32 (
{
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgl %3, %1 \n\t"
- : "=a" (CompareValue), // %0
- "=m" (*Value) // %1
- : "a" (CompareValue), // %2
- "r" (ExchangeValue), // %3
- "m" (*Value) // %4
+ "cmpxchgl %2, %1 \n\t"
+ : "+a" (CompareValue), // %0
+ "+m" (*Value) // %1
+ : "r" (ExchangeValue) // %2
: "memory",
"cc"
);
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64()
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
` (2 preceding siblings ...)
2018-09-29 22:23 ` [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32() Laszlo Ersek
@ 2018-09-29 22:23 ` Laszlo Ersek
2018-10-01 10:27 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64() Laszlo Ersek
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
(This patch is identical to the X64 half of the last one, except for the
InternalSyncCompareExchange32() -> InternalSyncCompareExchange64() and
"cmpxchgl" -> "cmpxchgq" replacements.)
The CMPXCHG instruction has the following operands:
- AX (implicit, CompareValue): input and output
- destination operand (*Value): input and output
- source operand (ExchangeValue): input
The X64 version of InternalSyncCompareExchange64() attempts to mark both
CompareValue and (*Value) as input/output, but it doesn't use the
appropriate constraints for either operand.
Fix these issues. Furthermore, prefer the short "+" constraint for I/O
operands over the <output-operand-number> constraint that can be applied
to the input instances of I/O operands.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
index a85cf0265c8b..edb904c00704 100644
--- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
@@ -194,12 +194,10 @@ InternalSyncCompareExchange64 (
{
__asm__ __volatile__ (
"lock \n\t"
- "cmpxchgq %3, %1 \n\t"
- : "=a" (CompareValue), // %0
- "=m" (*Value) // %1
- : "a" (CompareValue), // %2
- "r" (ExchangeValue), // %3
- "m" (*Value) // %4
+ "cmpxchgq %2, %1 \n\t"
+ : "+a" (CompareValue), // %0
+ "+m" (*Value) // %1
+ : "r" (ExchangeValue) // %2
: "memory",
"cc"
);
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
` (3 preceding siblings ...)
2018-09-29 22:23 ` [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64() Laszlo Ersek
@ 2018-09-29 22:23 ` Laszlo Ersek
2018-10-01 18:27 ` Philippe Mathieu-Daudé
2018-09-29 22:35 ` [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
2018-10-08 13:44 ` Laszlo Ersek
6 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:23 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
The IA32 variant of InternalSyncCompareExchange64() is correct, but we can
simplify it. We don't need to load the lower 32 bits of ExchangeValue into
EBX in two steps (first into a general register, then into EBX); we can
ask GCC to populate EBX like that itself.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
index 44188e265af2..af39bdeb516c 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
@@ -193,14 +193,11 @@ InternalSyncCompareExchange64 (
)
{
__asm__ __volatile__ (
- "push %%ebx \n\t"
- "movl %2,%%ebx \n\t"
"lock \n\t"
"cmpxchg8b (%1) \n\t"
- "pop %%ebx \n\t"
: "+A" (CompareValue) // %0
: "S" (Value), // %1
- "r" ((UINT32) ExchangeValue), // %2
+ "b" ((UINT32) ExchangeValue), // %2
"c" ((UINT32) (ExchangeValue >> 32)) // %3
: "memory",
"cc"
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
` (4 preceding siblings ...)
2018-09-29 22:23 ` [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64() Laszlo Ersek
@ 2018-09-29 22:35 ` Laszlo Ersek
2018-10-08 13:44 ` Laszlo Ersek
6 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2018-09-29 22:35 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 09/30/18 00:23, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: inline_asm_rw_ops_1208
>
> This series mainly fixes the operand constraints (missing input-output
> qualifications) in "BaseSynchronizationLib/*/GccInline.c".
>
> (It would be better to remove these files altogether in favor of the
> already existing NASM implementation, but due to
> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>, we can't generally
> do that in libraries yet.)
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (5):
> MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
> MdePkg/BaseSynchronizationLib GCC: fix X64
> InternalSyncCompareExchange64()
> MdePkg/BaseSynchronizationLib GCC: simplify IA32
> InternalSyncCompareExchange64()
>
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 42 +++++++----------
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 47 +++++++-------------
> 2 files changed, 34 insertions(+), 55 deletions(-)
>
Forgot to mention the testing.
Ran my usual OVMF tests described at
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt>
on multi-VCPU guests. I tested the series against all my longterm guests
(including 32-bit and 64-bit Fedora, and 64-bit Windows 7 / Server 2008
r2, 8.1 / Server 2012 r2, 10 / Server 2016). Also covered an SMM-less
build on i440fx, with 64-bit Fedora.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
@ 2018-10-01 10:17 ` Philippe Mathieu-Daudé
2018-10-18 2:00 ` Ni, Ruiyu
1 sibling, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-01 10:17 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 30/09/2018 00:23, Laszlo Ersek wrote:
> The "GccInline.c" files have some inconsistent whitespace, and missing (or
> incorrect) operand comments. Fix and unify them.
>
> This patch doesn't change behavior.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 35 ++++++-------
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 53 +++++++++-----------
> 2 files changed, 39 insertions(+), 49 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index fa2be7f4b35c..1976720ac636 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
> "movl $1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "inc %%eax "
> + "inc %%eax \n\t"
> : "=a" (Result), // %0
> "+m" (*Value) // %1
> : // no inputs that aren't also outputs
> @@ -48,7 +48,6 @@ InternalSyncIncrement (
> );
>
> return Result;
> -
> }
>
>
> @@ -76,10 +75,10 @@ InternalSyncDecrement (
> "movl $-1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "dec %%eax "
> - : "=a" (Result), // %0
> - "+m" (*Value) // %1
> - : // no inputs that aren't also outputs
> + "dec %%eax \n\t"
> + : "=a" (Result), // %0
> + "+m" (*Value) // %1
> + : // no inputs that aren't also outputs
> : "memory",
> "cc"
> );
> @@ -87,6 +86,7 @@ InternalSyncDecrement (
> return Result;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 16-bit unsigned integer.
>
> @@ -113,15 +113,13 @@ InternalSyncCompareExchange16 (
> IN UINT16 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> - " \n\t"
> "lock \n\t"
> "cmpxchgw %1, %2 \n\t"
> - : "=a" (CompareValue)
> - : "q" (ExchangeValue),
> - "m" (*Value),
> - "0" (CompareValue)
> + : "=a" (CompareValue) // %0
> + : "q" (ExchangeValue), // %1
> + "m" (*Value), // %2
> + "0" (CompareValue) // %3
> : "memory",
> "cc"
> );
> @@ -129,6 +127,7 @@ InternalSyncCompareExchange16 (
> return CompareValue;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 32-bit unsigned integer.
>
> @@ -155,15 +154,13 @@ InternalSyncCompareExchange32 (
> IN UINT32 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> - " \n\t"
> "lock \n\t"
> "cmpxchgl %1, %2 \n\t"
> - : "=a" (CompareValue) // %0
> - : "q" (ExchangeValue), // %1
> - "m" (*Value), // %2
> - "0" (CompareValue) // %4
> + : "=a" (CompareValue) // %0
> + : "q" (ExchangeValue), // %1
> + "m" (*Value), // %2
> + "0" (CompareValue) // %3
> : "memory",
> "cc"
> );
> @@ -171,6 +168,7 @@ InternalSyncCompareExchange32 (
> return CompareValue;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 64-bit unsigned integer.
>
> @@ -197,7 +195,6 @@ InternalSyncCompareExchange64 (
> )
> {
> __asm__ __volatile__ (
> - " \n\t"
> "push %%ebx \n\t"
> "movl %2,%%ebx \n\t"
> "lock \n\t"
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index ab7efe23c4db..0212798d7a27 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
> "movl $1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "inc %%eax "
> + "inc %%eax \n\t"
> : "=a" (Result), // %0
> "+m" (*Value) // %1
> : // no inputs that aren't also outputs
> @@ -75,10 +75,10 @@ InternalSyncDecrement (
> "movl $-1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "dec %%eax "
> - : "=a" (Result), // %0
> - "+m" (*Value) // %1
> - : // no inputs that aren't also outputs
> + "dec %%eax \n\t"
> + : "=a" (Result), // %0
> + "+m" (*Value) // %1
> + : // no inputs that aren't also outputs
> : "memory",
> "cc"
> );
> @@ -113,16 +113,14 @@ InternalSyncCompareExchange16 (
> IN UINT16 ExchangeValue
> )
> {
> -
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgw %3, %1 "
> - : "=a" (CompareValue),
> - "=m" (*Value)
> - : "a" (CompareValue),
> - "r" (ExchangeValue),
> - "m" (*Value)
> + "cmpxchgw %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
> @@ -157,16 +155,14 @@ InternalSyncCompareExchange32 (
> IN UINT32 ExchangeValue
> )
> {
> -
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgl %3, %1 "
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value)
> + "cmpxchgl %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
> @@ -200,20 +196,17 @@ InternalSyncCompareExchange64 (
> IN UINT64 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgq %3, %1 "
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value)
> + "cmpxchgq %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
>
> return CompareValue;
> }
> -
> -
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
2018-09-29 22:23 ` [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16() Laszlo Ersek
@ 2018-10-01 10:26 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-01 10:26 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 30/09/2018 00:23, Laszlo Ersek wrote:
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue): input and output
> - destination operand (*Value): input and output
> - source operand (ExchangeValue): input
>
> The IA32 version of InternalSyncCompareExchange16() correctly marks
> CompareValue as input/output, but it marks (*Value) only as input.
>
> The X64 version of InternalSyncCompareExchange16() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
>
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the <output-operand-number> constraint that can be applied
> to the input instances of I/O operands.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 9 ++++-----
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index 1976720ac636..bd98a5aebfe7 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -115,11 +115,10 @@ InternalSyncCompareExchange16 (
> {
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgw %1, %2 \n\t"
> - : "=a" (CompareValue) // %0
> - : "q" (ExchangeValue), // %1
> - "m" (*Value), // %2
> - "0" (CompareValue) // %3
> + "cmpxchgw %2, %1 \n\t"
> + : "+a" (CompareValue), // %0
> + "+m" (*Value) // %1
> + : "q" (ExchangeValue) // %2
> : "memory",
> "cc"
> );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index 0212798d7a27..4338fb8e65e8 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -115,12 +115,10 @@ InternalSyncCompareExchange16 (
> {
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgw %3, %1 \n\t"
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value) // %4
> + "cmpxchgw %2, %1 \n\t"
> + : "+a" (CompareValue), // %0
> + "+m" (*Value) // %1
> + : "r" (ExchangeValue) // %2
> : "memory",
> "cc"
> );
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
2018-09-29 22:23 ` [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32() Laszlo Ersek
@ 2018-10-01 10:26 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-01 10:26 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 30/09/2018 00:23, Laszlo Ersek wrote:
> (This patch is identical to the last one, except for the
> InternalSyncCompareExchange16() -> InternalSyncCompareExchange32() and
> "cmpxchgw" -> "cmpxchgl" replacements.)
>
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue): input and output
> - destination operand (*Value): input and output
> - source operand (ExchangeValue): input
>
> The IA32 version of InternalSyncCompareExchange32() correctly marks
> CompareValue as input/output, but it marks (*Value) only as input.
>
> The X64 version of InternalSyncCompareExchange32() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
>
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the <output-operand-number> constraint that can be applied
> to the input instances of I/O operands.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 9 ++++-----
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index bd98a5aebfe7..44188e265af2 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -155,11 +155,10 @@ InternalSyncCompareExchange32 (
> {
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgl %1, %2 \n\t"
> - : "=a" (CompareValue) // %0
> - : "q" (ExchangeValue), // %1
> - "m" (*Value), // %2
> - "0" (CompareValue) // %3
> + "cmpxchgl %2, %1 \n\t"
> + : "+a" (CompareValue), // %0
> + "+m" (*Value) // %1
> + : "q" (ExchangeValue) // %2
> : "memory",
> "cc"
> );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index 4338fb8e65e8..a85cf0265c8b 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -155,12 +155,10 @@ InternalSyncCompareExchange32 (
> {
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgl %3, %1 \n\t"
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value) // %4
> + "cmpxchgl %2, %1 \n\t"
> + : "+a" (CompareValue), // %0
> + "+m" (*Value) // %1
> + : "r" (ExchangeValue) // %2
> : "memory",
> "cc"
> );
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64()
2018-09-29 22:23 ` [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64() Laszlo Ersek
@ 2018-10-01 10:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-01 10:27 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 30/09/2018 00:23, Laszlo Ersek wrote:
> (This patch is identical to the X64 half of the last one, except for the
> InternalSyncCompareExchange32() -> InternalSyncCompareExchange64() and
> "cmpxchgl" -> "cmpxchgq" replacements.)
>
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue): input and output
> - destination operand (*Value): input and output
> - source operand (ExchangeValue): input
>
> The X64 version of InternalSyncCompareExchange64() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
>
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the <output-operand-number> constraint that can be applied
> to the input instances of I/O operands.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index a85cf0265c8b..edb904c00704 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -194,12 +194,10 @@ InternalSyncCompareExchange64 (
> {
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgq %3, %1 \n\t"
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value) // %4
> + "cmpxchgq %2, %1 \n\t"
> + : "+a" (CompareValue), // %0
> + "+m" (*Value) // %1
> + : "r" (ExchangeValue) // %2
> : "memory",
> "cc"
> );
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()
2018-09-29 22:23 ` [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64() Laszlo Ersek
@ 2018-10-01 18:27 ` Philippe Mathieu-Daudé
2018-10-01 18:45 ` Laszlo Ersek
0 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-01 18:27 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 30/09/2018 00:23, Laszlo Ersek wrote:
> The IA32 variant of InternalSyncCompareExchange64() is correct, but we can
> simplify it. We don't need to load the lower 32 bits of ExchangeValue into
> EBX in two steps (first into a general register, then into EBX); we can
> ask GCC to populate EBX like that itself.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index 44188e265af2..af39bdeb516c 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -193,14 +193,11 @@ InternalSyncCompareExchange64 (
> )
> {
> __asm__ __volatile__ (
> - "push %%ebx \n\t"
> - "movl %2,%%ebx \n\t"
> "lock \n\t"
> "cmpxchg8b (%1) \n\t"
> - "pop %%ebx \n\t"
> : "+A" (CompareValue) // %0
> : "S" (Value), // %1
> - "r" ((UINT32) ExchangeValue), // %2
> + "b" ((UINT32) ExchangeValue), // %2
> "c" ((UINT32) (ExchangeValue >> 32)) // %3
> : "memory",
> "cc"
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()
2018-10-01 18:27 ` Philippe Mathieu-Daudé
@ 2018-10-01 18:45 ` Laszlo Ersek
0 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2018-10-01 18:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 10/01/18 20:27, Philippe Mathieu-Daudé wrote:
> On 30/09/2018 00:23, Laszlo Ersek wrote:
>> The IA32 variant of InternalSyncCompareExchange64() is correct, but we can
>> simplify it. We don't need to load the lower 32 bits of ExchangeValue into
>> EBX in two steps (first into a general register, then into EBX); we can
>> ask GCC to populate EBX like that itself.
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>> ---
>> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> index 44188e265af2..af39bdeb516c 100644
>> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> @@ -193,14 +193,11 @@ InternalSyncCompareExchange64 (
>> )
>> {
>> __asm__ __volatile__ (
>> - "push %%ebx \n\t"
>> - "movl %2,%%ebx \n\t"
>> "lock \n\t"
>> "cmpxchg8b (%1) \n\t"
>> - "pop %%ebx \n\t"
>> : "+A" (CompareValue) // %0
>> : "S" (Value), // %1
>> - "r" ((UINT32) ExchangeValue), // %2
>> + "b" ((UINT32) ExchangeValue), // %2
>> "c" ((UINT32) (ExchangeValue >> 32)) // %3
>> : "memory",
>> "cc"
>>
Thank you for the review!
Laszlo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
` (5 preceding siblings ...)
2018-09-29 22:35 ` [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
@ 2018-10-08 13:44 ` Laszlo Ersek
2018-10-15 18:04 ` Laszlo Ersek
6 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-10-08 13:44 UTC (permalink / raw)
To: Michael D Kinney, Liming Gao; +Cc: edk2-devel-01
On 09/30/18 00:23, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: inline_asm_rw_ops_1208
>
> This series mainly fixes the operand constraints (missing input-output
> qualifications) in "BaseSynchronizationLib/*/GccInline.c".
>
> (It would be better to remove these files altogether in favor of the
> already existing NASM implementation, but due to
> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>, we can't generally
> do that in libraries yet.)
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (5):
> MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
> MdePkg/BaseSynchronizationLib GCC: fix X64
> InternalSyncCompareExchange64()
> MdePkg/BaseSynchronizationLib GCC: simplify IA32
> InternalSyncCompareExchange64()
>
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 42 +++++++----------
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 47 +++++++-------------
> 2 files changed, 34 insertions(+), 55 deletions(-)
>
Ping :)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-10-08 13:44 ` Laszlo Ersek
@ 2018-10-15 18:04 ` Laszlo Ersek
2018-10-15 19:29 ` Kinney, Michael D
0 siblings, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-10-15 18:04 UTC (permalink / raw)
To: Michael D Kinney, Liming Gao; +Cc: edk2-devel-01
On 10/08/18 15:44, Laszlo Ersek wrote:
> On 09/30/18 00:23, Laszlo Ersek wrote:
>> Repo: https://github.com/lersek/edk2.git
>> Branch: inline_asm_rw_ops_1208
>>
>> This series mainly fixes the operand constraints (missing input-output
>> qualifications) in "BaseSynchronizationLib/*/GccInline.c".
>>
>> (It would be better to remove these files altogether in favor of the
>> already existing NASM implementation, but due to
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>, we can't generally
>> do that in libraries yet.)
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (5):
>> MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
>> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()
>> MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()
>> MdePkg/BaseSynchronizationLib GCC: fix X64
>> InternalSyncCompareExchange64()
>> MdePkg/BaseSynchronizationLib GCC: simplify IA32
>> InternalSyncCompareExchange64()
>>
>> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 42 +++++++----------
>> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 47 +++++++-------------
>> 2 files changed, 34 insertions(+), 55 deletions(-)
>>
>
> Ping :)
Ping
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-10-15 18:04 ` Laszlo Ersek
@ 2018-10-15 19:29 ` Kinney, Michael D
2018-10-16 1:32 ` Gao, Liming
0 siblings, 1 reply; 19+ messages in thread
From: Kinney, Michael D @ 2018-10-15 19:29 UTC (permalink / raw)
To: Laszlo Ersek, Gao, Liming, Kinney, Michael D; +Cc: edk2-devel-01
Laszlo,
Thanks for the reminder. My knowledge of inline
GCC assembly syntax is very limited, so I am not
able to review this patch for correctness. I can
ack it.
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
Perhaps Liming can do a more complete review.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Monday, October 15, 2018 11:05 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH 0/5]
> MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
>
> On 10/08/18 15:44, Laszlo Ersek wrote:
> > On 09/30/18 00:23, Laszlo Ersek wrote:
> >> Repo: https://github.com/lersek/edk2.git
> >> Branch: inline_asm_rw_ops_1208
> >>
> >> This series mainly fixes the operand constraints
> (missing input-output
> >> qualifications) in
> "BaseSynchronizationLib/*/GccInline.c".
> >>
> >> (It would be better to remove these files altogether
> in favor of the
> >> already existing NASM implementation, but due to
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>,
> we can't generally
> >> do that in libraries yet.)
> >>
> >> Cc: Liming Gao <liming.gao@intel.com>
> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>
> >> Thanks,
> >> Laszlo
> >>
> >> Laszlo Ersek (5):
> >> MdePkg/BaseSynchronizationLib GCC: fix whitespace
> and comments
> >> MdePkg/BaseSynchronizationLib GCC: fix
> InternalSyncCompareExchange16()
> >> MdePkg/BaseSynchronizationLib GCC: fix
> InternalSyncCompareExchange32()
> >> MdePkg/BaseSynchronizationLib GCC: fix X64
> >> InternalSyncCompareExchange64()
> >> MdePkg/BaseSynchronizationLib GCC: simplify IA32
> >> InternalSyncCompareExchange64()
> >>
> >>
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c |
> 42 +++++++----------
> >>
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c |
> 47 +++++++-------------
> >> 2 files changed, 34 insertions(+), 55 deletions(-)
> >>
> >
> > Ping :)
>
> Ping
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-10-15 19:29 ` Kinney, Michael D
@ 2018-10-16 1:32 ` Gao, Liming
2018-10-17 17:16 ` Laszlo Ersek
0 siblings, 1 reply; 19+ messages in thread
From: Gao, Liming @ 2018-10-16 1:32 UTC (permalink / raw)
To: Kinney, Michael D, Laszlo Ersek; +Cc: edk2-devel-01
Laszlo:
Sorry for the delay. Your change is good.
Reviewed-by: Liming Gao <liming.gao@intel.com>
Thanks
Liming
>-----Original Message-----
>From: Kinney, Michael D
>Sent: Tuesday, October 16, 2018 3:29 AM
>To: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>
>Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>Subject: RE: [edk2] [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes,
>cleanups
>
>Laszlo,
>
>Thanks for the reminder. My knowledge of inline
>GCC assembly syntax is very limited, so I am not
>able to review this patch for correctness. I can
>ack it.
>
>Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
>
>Perhaps Liming can do a more complete review.
>
>Mike
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-
>> bounces@lists.01.org] On Behalf Of Laszlo Ersek
>> Sent: Monday, October 15, 2018 11:05 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
>> Liming <liming.gao@intel.com>
>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>> Subject: Re: [edk2] [PATCH 0/5]
>> MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
>>
>> On 10/08/18 15:44, Laszlo Ersek wrote:
>> > On 09/30/18 00:23, Laszlo Ersek wrote:
>> >> Repo: https://github.com/lersek/edk2.git
>> >> Branch: inline_asm_rw_ops_1208
>> >>
>> >> This series mainly fixes the operand constraints
>> (missing input-output
>> >> qualifications) in
>> "BaseSynchronizationLib/*/GccInline.c".
>> >>
>> >> (It would be better to remove these files altogether
>> in favor of the
>> >> already existing NASM implementation, but due to
>> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>,
>> we can't generally
>> >> do that in libraries yet.)
>> >>
>> >> Cc: Liming Gao <liming.gao@intel.com>
>> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> >>
>> >> Thanks,
>> >> Laszlo
>> >>
>> >> Laszlo Ersek (5):
>> >> MdePkg/BaseSynchronizationLib GCC: fix whitespace
>> and comments
>> >> MdePkg/BaseSynchronizationLib GCC: fix
>> InternalSyncCompareExchange16()
>> >> MdePkg/BaseSynchronizationLib GCC: fix
>> InternalSyncCompareExchange32()
>> >> MdePkg/BaseSynchronizationLib GCC: fix X64
>> >> InternalSyncCompareExchange64()
>> >> MdePkg/BaseSynchronizationLib GCC: simplify IA32
>> >> InternalSyncCompareExchange64()
>> >>
>> >>
>> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c |
>> 42 +++++++----------
>> >>
>> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c |
>> 47 +++++++-------------
>> >> 2 files changed, 34 insertions(+), 55 deletions(-)
>> >>
>> >
>> > Ping :)
>>
>> Ping
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
2018-10-16 1:32 ` Gao, Liming
@ 2018-10-17 17:16 ` Laszlo Ersek
0 siblings, 0 replies; 19+ messages in thread
From: Laszlo Ersek @ 2018-10-17 17:16 UTC (permalink / raw)
To: Gao, Liming, Kinney, Michael D; +Cc: edk2-devel-01, Philippe Mathieu-Daudé
On 10/16/18 03:32, Gao, Liming wrote:
> Laszlo:
> Sorry for the delay. Your change is good.
>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
>
> Thanks
> Liming
>> -----Original Message-----
>> From: Kinney, Michael D
>> Sent: Tuesday, October 16, 2018 3:29 AM
>> To: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>> Subject: RE: [edk2] [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes,
>> cleanups
>>
>> Laszlo,
>>
>> Thanks for the reminder. My knowledge of inline
>> GCC assembly syntax is very limited, so I am not
>> able to review this patch for correctness. I can
>> ack it.
>>
>> Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
>>
>> Perhaps Liming can do a more complete review.
Thank you both. This is a very busy time for the edk2 project &
community; it's expected that reviews take longer. I didn't intend my
pings as urging you, just as friendly reminders, once per week :)
Series pushed as commit range b7dc8888f314..3a0329bed2a2.
Thanks!
Laszlo
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-
>>> bounces@lists.01.org] On Behalf Of Laszlo Ersek
>>> Sent: Monday, October 15, 2018 11:05 AM
>>> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
>>> Liming <liming.gao@intel.com>
>>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>>> Subject: Re: [edk2] [PATCH 0/5]
>>> MdePkg/BaseSynchronizationLib GCC: fixes, cleanups
>>>
>>> On 10/08/18 15:44, Laszlo Ersek wrote:
>>>> On 09/30/18 00:23, Laszlo Ersek wrote:
>>>>> Repo: https://github.com/lersek/edk2.git
>>>>> Branch: inline_asm_rw_ops_1208
>>>>>
>>>>> This series mainly fixes the operand constraints
>>> (missing input-output
>>>>> qualifications) in
>>> "BaseSynchronizationLib/*/GccInline.c".
>>>>>
>>>>> (It would be better to remove these files altogether
>>> in favor of the
>>>>> already existing NASM implementation, but due to
>>>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=881>,
>>> we can't generally
>>>>> do that in libraries yet.)
>>>>>
>>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>>>
>>>>> Thanks,
>>>>> Laszlo
>>>>>
>>>>> Laszlo Ersek (5):
>>>>> MdePkg/BaseSynchronizationLib GCC: fix whitespace
>>> and comments
>>>>> MdePkg/BaseSynchronizationLib GCC: fix
>>> InternalSyncCompareExchange16()
>>>>> MdePkg/BaseSynchronizationLib GCC: fix
>>> InternalSyncCompareExchange32()
>>>>> MdePkg/BaseSynchronizationLib GCC: fix X64
>>>>> InternalSyncCompareExchange64()
>>>>> MdePkg/BaseSynchronizationLib GCC: simplify IA32
>>>>> InternalSyncCompareExchange64()
>>>>>
>>>>>
>>> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c |
>>> 42 +++++++----------
>>>>>
>>> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c |
>>> 47 +++++++-------------
>>>>> 2 files changed, 34 insertions(+), 55 deletions(-)
>>>>>
>>>>
>>>> Ping :)
>>>
>>> Ping
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
2018-10-01 10:17 ` Philippe Mathieu-Daudé
@ 2018-10-18 2:00 ` Ni, Ruiyu
1 sibling, 0 replies; 19+ messages in thread
From: Ni, Ruiyu @ 2018-10-18 2:00 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Michael D Kinney, Liming Gao
On 9/30/2018 6:23 AM, Laszlo Ersek wrote:
> The "GccInline.c" files have some inconsistent whitespace, and missing (or
> incorrect) operand comments. Fix and unify them.
>
> This patch doesn't change behavior.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 35 ++++++-------
> MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 53 +++++++++-----------
> 2 files changed, 39 insertions(+), 49 deletions(-)
>
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index fa2be7f4b35c..1976720ac636 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
> "movl $1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "inc %%eax "
> + "inc %%eax \n\t"
> : "=a" (Result), // %0
> "+m" (*Value) // %1
> : // no inputs that aren't also outputs
> @@ -48,7 +48,6 @@ InternalSyncIncrement (
> );
>
> return Result;
> -
> }
>
>
> @@ -76,10 +75,10 @@ InternalSyncDecrement (
> "movl $-1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "dec %%eax "
> - : "=a" (Result), // %0
> - "+m" (*Value) // %1
> - : // no inputs that aren't also outputs
> + "dec %%eax \n\t"
> + : "=a" (Result), // %0
> + "+m" (*Value) // %1
> + : // no inputs that aren't also outputs
> : "memory",
> "cc"
> );
> @@ -87,6 +86,7 @@ InternalSyncDecrement (
> return Result;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 16-bit unsigned integer.
>
> @@ -113,15 +113,13 @@ InternalSyncCompareExchange16 (
> IN UINT16 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> - " \n\t"
> "lock \n\t"
> "cmpxchgw %1, %2 \n\t"
> - : "=a" (CompareValue)
> - : "q" (ExchangeValue),
> - "m" (*Value),
> - "0" (CompareValue)
> + : "=a" (CompareValue) // %0
> + : "q" (ExchangeValue), // %1
> + "m" (*Value), // %2
> + "0" (CompareValue) // %3
> : "memory",
> "cc"
> );
> @@ -129,6 +127,7 @@ InternalSyncCompareExchange16 (
> return CompareValue;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 32-bit unsigned integer.
>
> @@ -155,15 +154,13 @@ InternalSyncCompareExchange32 (
> IN UINT32 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> - " \n\t"
> "lock \n\t"
> "cmpxchgl %1, %2 \n\t"
> - : "=a" (CompareValue) // %0
> - : "q" (ExchangeValue), // %1
> - "m" (*Value), // %2
> - "0" (CompareValue) // %4
> + : "=a" (CompareValue) // %0
> + : "q" (ExchangeValue), // %1
> + "m" (*Value), // %2
> + "0" (CompareValue) // %3
> : "memory",
> "cc"
> );
> @@ -171,6 +168,7 @@ InternalSyncCompareExchange32 (
> return CompareValue;
> }
>
> +
> /**
> Performs an atomic compare exchange operation on a 64-bit unsigned integer.
>
> @@ -197,7 +195,6 @@ InternalSyncCompareExchange64 (
> )
> {
> __asm__ __volatile__ (
> - " \n\t"
> "push %%ebx \n\t"
> "movl %2,%%ebx \n\t"
> "lock \n\t"
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index ab7efe23c4db..0212798d7a27 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
> "movl $1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "inc %%eax "
> + "inc %%eax \n\t"
> : "=a" (Result), // %0
> "+m" (*Value) // %1
> : // no inputs that aren't also outputs
> @@ -75,10 +75,10 @@ InternalSyncDecrement (
> "movl $-1, %%eax \n\t"
> "lock \n\t"
> "xadd %%eax, %1 \n\t"
> - "dec %%eax "
> - : "=a" (Result), // %0
> - "+m" (*Value) // %1
> - : // no inputs that aren't also outputs
> + "dec %%eax \n\t"
> + : "=a" (Result), // %0
> + "+m" (*Value) // %1
> + : // no inputs that aren't also outputs
> : "memory",
> "cc"
> );
> @@ -113,16 +113,14 @@ InternalSyncCompareExchange16 (
> IN UINT16 ExchangeValue
> )
> {
> -
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgw %3, %1 "
> - : "=a" (CompareValue),
> - "=m" (*Value)
> - : "a" (CompareValue),
> - "r" (ExchangeValue),
> - "m" (*Value)
> + "cmpxchgw %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
> @@ -157,16 +155,14 @@ InternalSyncCompareExchange32 (
> IN UINT32 ExchangeValue
> )
> {
> -
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgl %3, %1 "
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value)
> + "cmpxchgl %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
> @@ -200,20 +196,17 @@ InternalSyncCompareExchange64 (
> IN UINT64 ExchangeValue
> )
> {
> -
> __asm__ __volatile__ (
> "lock \n\t"
> - "cmpxchgq %3, %1 "
> - : "=a" (CompareValue), // %0
> - "=m" (*Value) // %1
> - : "a" (CompareValue), // %2
> - "r" (ExchangeValue), // %3
> - "m" (*Value)
> + "cmpxchgq %3, %1 \n\t"
> + : "=a" (CompareValue), // %0
> + "=m" (*Value) // %1
> + : "a" (CompareValue), // %2
> + "r" (ExchangeValue), // %3
> + "m" (*Value) // %4
> : "memory",
> "cc"
> );
>
> return CompareValue;
> }
> -
> -
>
Thanks!
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-10-18 1:59 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-29 22:23 [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
2018-09-29 22:23 ` [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments Laszlo Ersek
2018-10-01 10:17 ` Philippe Mathieu-Daudé
2018-10-18 2:00 ` Ni, Ruiyu
2018-09-29 22:23 ` [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16() Laszlo Ersek
2018-10-01 10:26 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32() Laszlo Ersek
2018-10-01 10:26 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64() Laszlo Ersek
2018-10-01 10:27 ` Philippe Mathieu-Daudé
2018-09-29 22:23 ` [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64() Laszlo Ersek
2018-10-01 18:27 ` Philippe Mathieu-Daudé
2018-10-01 18:45 ` Laszlo Ersek
2018-09-29 22:35 ` [PATCH 0/5] MdePkg/BaseSynchronizationLib GCC: fixes, cleanups Laszlo Ersek
2018-10-08 13:44 ` Laszlo Ersek
2018-10-15 18:04 ` Laszlo Ersek
2018-10-15 19:29 ` Kinney, Michael D
2018-10-16 1:32 ` Gao, Liming
2018-10-17 17:16 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox