This is an archive of the discontinued LLVM Phabricator instance.

LLVM OpenMP Backend -- Fix "static chunked" scheduling
ClosedPublic

Authored by mhalk on Apr 24 2019, 12:03 PM.

Details

Summary

Static chunked OpenMP scheduling has not been treated correctly.
This patch fixes the problem that threads would not process their (work-)chunks as intended.

Diff Detail

Event Timeline

mhalk created this revision.Apr 24 2019, 12:03 PM
Meinersbur added inline comments.Apr 24 2019, 12:49 PM
lib/CodeGen/LoopGeneratorsGOMP.cpp
76–80 ↗(On Diff #196490)

[nit] unrelated?

lib/CodeGen/LoopGeneratorsKMP.cpp
219–220

Are these renames related to the patch? If not, can you undo them? It would make the diff smaller, especially reduces the changed lines in the test case to the ones that are relevant.

test/Isl/CodeGen/OpenMP/single_loop.ll
7–8

[serious] Since they are significantly different, there should be a test for -polly-scheduling=static without chunksize as well.

mhalk updated this revision to Diff 196529.Apr 24 2019, 3:03 PM

Incorporated feedback from @Meinersbur

mhalk marked 4 inline comments as done.Apr 24 2019, 3:11 PM
mhalk added inline comments.
lib/CodeGen/LoopGeneratorsKMP.cpp
219–220

Not really, I wanted to be closer to the naming of the original backend.
Reverted the names; now the diff is smaller and "more clearly".

test/Isl/CodeGen/OpenMP/single_loop.ll
7–8

Done. (Thought about it, too.)
Note: In the setup BB (which is the same for both variants), there's only a check for the right init-call and branch for the 'non-chunked' version.

mhalk marked 2 inline comments as done.May 14 2019, 11:30 AM

ping

Meinersbur accepted this revision.May 21 2019, 11:37 AM

LGTM. Can I commit for you?

Sorry for the delay, I was really busy the last weeks.

This revision is now accepted and ready to land.May 21 2019, 11:37 AM

LGTM. Can I commit for you?

That would be great -- thank you!
(Hopefully, I catched everything this time :) )

Sorry for the delay, I was really busy the last weeks.

That's absolutely fine by me, no worries.
I just wanted to make sure this patch was not forgotten or overlooked.

Sorry, I completely missed that I did not commit this yet. Without ping, I would not have noticed. It's committed now. Thanks for your contribution!

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2020, 11:01 AM