Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpenMP] Use a memory fence before incrementing the dispatch buffer index #87995

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

xingxue-ibm
Copy link
Contributor

This patch uses a memory fence in function __kmp_dispatch_next() to flush pending memory write invalidates before incrementing the volatile variable buffer_index to fix intermittent time-outs of OpenMP runtime LIT test cases env/kmp_set_dispatch_buf.c and worksharing/for/kmp_set_dispatch_buf.c, noting that the same is needed for incrementing buffer_index in function __kmpc_next_section() (line 2600 of kmp_dispatch.cpp).

@xingxue-ibm xingxue-ibm added the openmp:libomp OpenMP host runtime label Apr 8, 2024
@xingxue-ibm xingxue-ibm self-assigned this Apr 8, 2024
@xingxue-ibm xingxue-ibm requested a review from jprotze April 9, 2024 20:27
Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

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

LGTM

@jprotze
Copy link
Collaborator

jprotze commented Apr 10, 2024

It is not clear to me what effect the memory barrier is supposed to have at this specific place with the oder two barriers a few lines above and below.

From my perspective the code shouldn't use volatile at all, but atomics where necessary.

@xingxue-ibm
Copy link
Contributor Author

It is not clear to me what effect the memory barrier is supposed to have at this specific place with the oder two barriers a few lines above and below.

From my perspective the code shouldn't use volatile at all, but atomics where necessary.

Yeah, atomics probably works better and it is a good candidate for the future improvement.

@xingxue-ibm xingxue-ibm merged commit 454d449 into llvm:main Apr 16, 2024
6 checks passed
@xingxue-ibm xingxue-ibm deleted the dispatch-buf-index branch May 24, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants