Re: [edk2] [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()

2018-10-01 Thread Philippe Mathieu-Daudé 
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  constraint that can be applied
> to the input instances of I/O operands.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  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"
>  );
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()

2018-09-29 Thread Laszlo Ersek 
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  constraint that can be applied
to the input instances of I/O operands.

Cc: Liming Gao 
Cc: Michael D Kinney 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 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


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel