<div dir="ltr"><div class="gmail_default" style="font-family:georgia,serif;color:rgb(0,0,0)">It is just counting cpusPerNode, so the 64-bit number is not required, yes but I missed the fact of support on few CPUs. Lookup table based implementation could have been fastest due to better caching, but it is not used frequently so we can keep as it is. Thanks.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 3, 2018 at 11:24 PM, Andrey Semashev <span dir="ltr"><<a href="mailto:andrey.semashev@gmail.com" target="_blank">andrey.semashev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, May 3, 2018 at 7:37 PM, Pradeep Ramachandran<br>
<<a href="mailto:pradeep@multicorewareinc.com">pradeep@multicorewareinc.com</a>> wrote:<br>
><br>
> On Thu, May 3, 2018 at 2:23 PM, <<a href="mailto:praveen@multicorewareinc.com">praveen@multicorewareinc.com</a>> wrote:<br>
>><br>
>> # HG changeset patch<br>
>> # User Praveen Tiwari <<a href="mailto:praveen@multicorewareinc.com">praveen@multicorewareinc.com</a>><br>
>> # Date 1525328839 -19800<br>
>> # Thu May 03 11:57:19 2018 +0530<br>
>> # Branch stable<br>
>> # Node ID 9cbb2aadcca3a2f7a308ea1dc792fb<wbr>817bcc5b51<br>
>> # Parent 69aafa6d70ad4e151f4590766c6b12<wbr>5621c5d007<br>
>> threadpool.cpp: use WIN system call for popcount<br>
><br>
><br>
> Unless this fixes a known bug, I don't want to push this directly into<br>
> stable. Syscalls are notorious especially when working with older versions<br>
> of the OS.<br>
> I would rather push this into default and allow users to test that this<br>
> works with all kinds of systems and then merge with stable once the answer<br>
> is known.<br>
> Does this fix a specific issue on some platform, or improve performance?<br>
<br>
</span>The comment is not quite right, __popcnt is not a syscall but an<br>
MSVC-specific intrinsic.<br>
<br>
<a href="https://msdn.microsoft.com/en-us/library/bb385231.aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-<wbr>us/library/bb385231.aspx</a><br>
<br>
The equivalent gcc intrinsic is __builtin_popcount and friends.<br>
<br>
I think, the patch is buggy because the relevant field is a 64-bit<br>
integer on 64-bit Windows and __popcnt is 32-bit.<br>
<br>
Note also that the popcount instruction only available in ABM ISA<br>
extension. In Intel CPUs it is available since Nehalem.<br>
<div class="HOEnZb"><div class="h5"><br>
>> diff -r 69aafa6d70ad -r 9cbb2aadcca3 source/common/threadpool.cpp<br>
>> --- a/source/common/threadpool.cpp Wed May 02 15:15:05 2018 +0530<br>
>> +++ b/source/common/threadpool.cpp Thu May 03 11:57:19 2018 +0530<br>
>> @@ -71,21 +71,6 @@<br>
>> # define strcasecmp _stricmp<br>
>> #endif<br>
>><br>
>> -#if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7<br>
>> -const uint64_t m1 = 0x5555555555555555; //binary: 0101...<br>
>> -const uint64_t m2 = 0x3333333333333333; //binary: 00110011..<br>
>> -const uint64_t m3 = 0x0f0f0f0f0f0f0f0f; //binary: 4 zeros, 4 ones ...<br>
>> -const uint64_t h01 = 0x0101010101010101; //the sum of 256 to the power of<br>
>> 0,1,2,3...<br>
>> -<br>
>> -static int popCount(uint64_t x)<br>
>> -{<br>
>> - x -= (x >> 1) & m1;<br>
>> - x = (x & m2) + ((x >> 2) & m2);<br>
>> - x = (x + (x >> 4)) & m3;<br>
>> - return (x * h01) >> 56;<br>
>> -}<br>
>> -#endif<br>
>> -<br>
>> namespace X265_NS {<br>
>> // x265 private namespace<br>
>><br>
>> @@ -274,7 +259,7 @@<br>
>> for (int i = 0; i < numNumaNodes; i++)<br>
>> {<br>
>> GetNumaNodeProcessorMaskEx((<wbr>UCHAR)i, groupAffinityPointer);<br>
>> - cpusPerNode[i] = popCount(groupAffinityPointer-<wbr>>Mask);<br>
>> + cpusPerNode[i] = __popcnt(static_cast<unsigned<br>
>> int>(groupAffinityPointer-><wbr>Mask));<br>
>> }<br>
>> delete groupAffinityPointer;<br>
>> #elif HAVE_LIBNUMA<br>
>> @@ -623,7 +608,7 @@<br>
>> for (int i = 0; i < numNumaNodes; i++)<br>
>> {<br>
>> GetNumaNodeProcessorMaskEx((<wbr>UCHAR)i, &groupAffinity);<br>
>> - cpus += popCount(groupAffinity.Mask);<br>
>> + cpus += __popcnt(static_cast<unsigned int>(groupAffinity.Mask));<br>
>> }<br>
>> return cpus;<br>
>> #elif _WIN32<br>
>> ______________________________<wbr>_________________<br>
>> x265-devel mailing list<br>
>> <a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
>> <a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/<wbr>listinfo/x265-devel</a><br>
><br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> x265-devel mailing list<br>
> <a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
> <a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/<wbr>listinfo/x265-devel</a><br>
><br>
______________________________<wbr>_________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/<wbr>listinfo/x265-devel</a><br>
</div></div></blockquote></div><br></div>