Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix GetFullAffinityMask for cpuCount==64#23276

Merged
jkotas merged 2 commits intodotnet:masterfrom
janvorli:fix-get-full-affinity-mask
Mar 15, 2019
Merged

Fix GetFullAffinityMask for cpuCount==64#23276
jkotas merged 2 commits intodotnet:masterfrom
janvorli:fix-get-full-affinity-mask

Conversation

@janvorli
Copy link
Member

The function was incorrectly assuming that shifting 64 bit
constant 1 by 64 bits to the left gets result 0.

The function was incorrectly assuming that shifting 64 bit
constant 1 by 64 bits to the left gets result 0.
@janvorli janvorli added this to the 3.0 milestone Mar 15, 2019
@janvorli janvorli self-assigned this Mar 15, 2019
@janvorli janvorli requested a review from jkotas March 15, 2019 01:57
@janvorli
Copy link
Member Author

cc: @vkvenkat

{
return ((uintptr_t)1 << (cpuCount)) - 1;
assert((cpuCount > 0) && (cpuCount <= sizeof(uintptr_t) * 8));
return (((uintptr_t)1 << (cpuCount - 1)) << 1) - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not obvious why the shift is split into two parts. It can use a comment.

Also, it may be better for readability to just use if (cpuCount >= sizeof(uintptr_t) * 8) ....

@janvorli janvorli closed this Mar 15, 2019
@janvorli janvorli reopened this Mar 15, 2019
@vkvenkat
Copy link

@janvorli The smask & pmask are now set as expected for machines with cores > 64. In addition, this fixes the GC Heap count when using taskset for limiting cores for the last case here.

@jkotas jkotas merged commit 6600932 into dotnet:master Mar 15, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
The function was incorrectly assuming that shifting 64 bit
constant 1 by 64 bits to the left gets result 0.



Commit migrated from dotnet/coreclr@6600932
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants