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

[Laszlo Ersek <lersek@redhat.com>]

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