[HarfBuzz] Fixes for harfbuzz-ng on Windows MSVC2010

[Koji Ishii]

> Good.  So the only difference is the concrt.h inclusion.
> I assume I can include that unconditionally?

Yes.


-----Original Message-----
From: Behdad Esfahbod [mailto:behdad.esfahbod at gmail.com] On Behalf Of Behdad Esfahbod
Sent: Sunday, July 15, 2012 1:15 AM
To: Koji Ishii
Cc: harfbuzz at lists.freedesktop.org
Subject: Re: [HarfBuzz] Fixes for harfbuzz-ng on Windows MSVC2010

On 07/14/2012 01:21 AM, Koji Ishii wrote:
> Thank you Behdad for pushing the changes.
> 
>> Can you please confirm that in general the intrinsic pragmas are needed at all?
> 
> That's a good point. It looks to me that it doesn't do anything.
> 
> I removed the two "#pragma intrinsic" in hb-atomic-private.hh, but all config (x86/x64, debug/release) still compile and there's no diff in the asm output both in x86 and x64.
> 
> So I guess you can safely remove them.

Good.  So the only difference is the concrt.h inclusion.  I assume I can include that unconditionally?

b


> 
> Regards,
> Koji
> 
> -----Original Message-----
> From: Behdad Esfahbod [mailto:behdad.esfahbod at gmail.com] On Behalf Of 
> Behdad Esfahbod
> Sent: Friday, July 13, 2012 10:04 PM
> To: Koji Ishii
> Cc: harfbuzz at lists.freedesktop.org
> Subject: Re: [HarfBuzz] Fixes for harfbuzz-ng on Windows MSVC2010
> 
> Hi Koji,
> 
> Thanks.  Comments below.
> 
> On 07/12/2012 12:33 PM, Koji Ishii wrote:
>>
>>>> hb-atomic-private.hh fix is for x86 only. The current code builds 
>>>> fine for x64 but not for x86.
>>>
>>> I don't understand why this is needed.  According to:
>>>
>>>   http://msdn.microsoft.com/en-us/library/1b4s3xf5%28v=vs.80%29.aspx
>>>
>>> _InterlockedCompareExchangePointer is available on both x86 and x64.
>>
>> And the page says:
>> Note   On the x86 architecture, _InterlockedCompareExchangePointer is a macro that calls _InterlockedCompareExchange.
>>
>> So, you can't declare it as intrinsic, and the macro is defined in concrt.h, not in intrin.h.
> 
> I see...
> 
> Can you please confirm that in general the intrinsic pragmas are needed at all?
> 
> I'm pushed your suggested change out now.
> 
> behdad
> 
>> I guess I've got better fix thanks to your advice. How's this? This compiles good too, and we don't hard code the macro any longer. I still see some warnings on both x86/x64, but that's a separate issue.
>>
>> Regards,
>> Koji
>>
>> diff --git a/src/hb-atomic-private.hh b/src/hb-atomic-private.hh 
>> index
>> 918852d..3653608 100644
>> --- a/src/hb-atomic-private.hh
>> +++ b/src/hb-atomic-private.hh
>> @@ -45,7 +45,12 @@
>>  #elif !defined(HB_NO_MT) && defined(_MSC_VER) && _MSC_VER >= 1600
>>  
>>  #include <intrin.h>
>> +#if defined(_M_IX86)
>> +#include <concrt.h>
>> +#pragma intrinsic(_InterlockedExchangeAdd)
>> +#else
>>  #pragma intrinsic(_InterlockedExchangeAdd,
>> _InterlockedCompareExchangePointer)
>> +#endif
>>  
>>  typedef long hb_atomic_int_t;
>>  #define hb_atomic_int_add(AI, V)	_InterlockedExchangeAdd (&(AI), (V))
>>
>>
> 
>