Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

2018-10-01 Thread Laszlo Ersek 
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 
>> 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 | 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

2018-10-01 Thread Philippe Mathieu-Daudé 
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 
> 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 | 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"
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

2018-09-29 Thread Laszlo Ersek 
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 
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 | 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

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