[HarfBuzz] Fixes for harfbuzz-ng on Windows MSVC2010

[Koji Ishii]

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.


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))
> 
>