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:[email protected]] On Behalf Of Behdad > Esfahbod > Sent: Friday, July 13, 2012 10:04 PM > To: Koji Ishii > Cc: [email protected] > 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)) >> >> > > _______________________________________________ HarfBuzz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/harfbuzz
