This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Fix dynamic schedule assertion failure on AArch64
ClosedPublic

Authored by Nawrin on Dec 5 2022, 3:35 PM.

Details

Summary

This patch fixes assertion failure for dynamic schedule with 8 byte induction variable. It checks the steal lock of selected victim and if the lock is not initialized then pick a different victim to steal from.

The bug report: https://github.com/llvm/llvm-project/issues/54422#issuecomment-1274020428

Diff Detail

Event Timeline

Nawrin created this revision.Dec 5 2022, 3:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 3:35 PM
Nawrin requested review of this revision.Dec 5 2022, 3:35 PM
Nawrin retitled this revision from Fix dynamic schedule assertion failure on AArch64 to [OpenMP] Fix dynamic schedule assertion failure on AArch64.Dec 5 2022, 3:40 PM
AndreyChurbanov added inline comments.
openmp/runtime/src/kmp_dispatch.cpp
1293

This change fights with the consequence of the problem, not with the root cause. IMHO, the better fix would be to change

if (KMP_ATOMIC_LD_RLX(&v->steal_flag) != READY ||

to

if (KMP_ATOMIC_LD_ACQ(&v->steal_flag) != READY ||

Then the acquire load would synchronize with release store at buffer initialization, that should prevent the steal_lock pointer to ever be NULL here.

Nawrin updated this revision to Diff 488286.Jan 11 2023, 10:42 AM
Nawrin added a reviewer: AndreyChurbanov.
AndreyChurbanov accepted this revision.Jan 12 2023, 7:09 AM

LGTM.

This patch fixes the issue reported in https://github.com/llvm/llvm-project/issues/54422 for me.

This revision is now accepted and ready to land.Jan 12 2023, 7:09 AM
This revision was automatically updated to reflect the committed changes.