Skip to content

[OpenMP][Runtime][test] Fix ompt task testcase fail randomly #72337

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

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

MANGOPIE3
Copy link
Contributor

Fixed #72231

@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Nov 15, 2023
Copy link

github-actions bot commented Nov 15, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@shiltian shiltian requested a review from jprotze November 24, 2023 21:30
Copy link
Collaborator

@jprotze jprotze left a comment

Choose a reason for hiding this comment

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

I prefer a different solution, so that the relevant ordering constraint is enforced.

@@ -59,10 +59,8 @@ int main()
// CHECK-DAG: {{^}}[[WORKER_ID:[0-9]+]]: ompt_event_implicit_task_begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer a different fix: Remove all the MASTER_ID lines, change this line:

Suggested change
// CHECK-DAG: {{^}}[[WORKER_ID:[0-9]+]]: ompt_event_implicit_task_begin
// CHECK: {{^}}[[WORKER_ID:[0-9]+]]: ompt_event_implicit_task_begin{{.*}}thread_num=1

And revert the changes below.

If the line becomes to long for clang-format, moving the CHECK lines to the end of the file will safe 2 trailing white spaces, WORKER_ID-> WID will safe another 6 characters, or change ompt_event_ -> {{.*}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review and advice, I've fixed it already.

@MANGOPIE3 MANGOPIE3 force-pushed the main branch 6 times, most recently from 0306f4a to 0a7d6ae Compare November 28, 2023 10:37
@jprotze
Copy link
Collaborator

jprotze commented Nov 28, 2023

Thanks for cleaning up the patch. LGTM now

@jprotze jprotze merged commit d6f0065 into llvm:main Nov 28, 2023
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.

[openmp][runtime][test] ompt test case failed occasionally
3 participants