Page MenuHomePhabricator

[OpenMP] Enabling CPU affinity on Darwin platform proposal
Needs ReviewPublic

Authored by devnexen on Jan 1 2020, 11:38 AM.

Diff Detail

Event Timeline

devnexen created this revision.Jan 1 2020, 11:38 AM
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert added a subscriber: protze.joachim.

I can't test this but it looks reasonable, @protze.joachim what do you think?

As Johannes, I cannot test this. The code seems reasonable.
Please apply clang-format on the changes.

Something like:

git clang-format HEAD~
openmp/runtime/src/kmp_affinity.h
317

The formatting looks wrong here.

please apply git clang-format

devnexen updated this revision to Diff 239055.Jan 20 2020, 3:22 AM
openmp/runtime/src/kmp_affinity.h
313

IINM, if the def is true, then thread_policy_get will always return default policy, which is 0. Is it the intention here?

321

The affinity_tag is treated as kind of bit mask here. But actually the affinity_tag is just an ID which hints the system to keep closer threads with same tags, and keep spread threads with different tags.
Of cause it can be used as kind of mask anyway, if set appropriately in set_system_affinity.

And I am not sure the thread_policy_get can work with array of policies. From what I saw in public examples, it works with single policy, and cnt parameter is just the number of integers in the corresponding structure, not the size of array.

355

Same as for thread_policy_get, I think the thread_policy_set works with single policy, not with array of them. Thus the third parameter should be thread_affinity_policy_data_t*, not **. At least this is how public examples on thread_policy_set/get are written (though I can mistake here as I am not an expert in Darwin affinity).

openmp/runtime/src/z_Linux_util.cpp
130

If I read thread_policy.h correctly, the THREAD_AFFINITY_POLICY_COUNT is the number of integers in the thread_affinity_policy_data_t structure, which is 1. This does not relate with the size of CPU_SET.

301

The buf is array of size 1 byte here. Whereas the thread_policy_get writes 4 bytes (sizeof thread_affinity_policy_data_t structure) into buf.

308

Parameter to KMP_AFFINITY_ENABLE is the size of affinity mask. It can be 1 if __kmp_affin_mask_size is only used to indicate affinity capable, and the mask is not used as a bit mask. Or if there is single proc available on teh system that is unlikely nowadays.

devnexen updated this revision to Diff 239492.Jan 21 2020, 10:13 PM