Primary Author: @anchu-rajendran
This patch adds section support in the OpenMP IRBuilder module. Also added a test for it.
Differential D89671
[LLVM][OpenMP] Adding support for OpenMP sections construct in OpenMPIRBuilder AMDChirag on Oct 19 2020, 12:39 AM. Authored by
Details Primary Author: @anchu-rajendran This patch adds section support in the OpenMP IRBuilder module. Also added a test for it.
Diff Detail
Event TimelineThere are a very large number of changes, so older changes are hidden. Show Older Changes This comment was removed by AMDChirag. Comment Actions Amended the original commit, applying the fixes:
Comment Actions Some drive-by comments. I will assume @fghanim continues the review, feel free to not wait on me.
Comment Actions Great work. Thanks. Also, please add the revision where this is used in clang as a child revision. It's a great validity check for this, and also showcases the intended way this is expected to be used. Up to this point I have been guessing the intent.
Comment Actions Applied some review changes Moved the scheduler type enum to OpenMPKinds.def, inlined Comment Actions The diff is against the previous version, not against a base version in LLVM. I usually squash all changes into a single commit but there are other ways too.
Comment Actions One idea: Now with D90830 landed, CreateSections could be implemented by calling that (with TripCount == Number of sections), then call "CreateWorkshareLoop" (the same that we would use to implement #pragma omp for) to it, this would allow us to share the implementations of worksharing-loop and sections. CreateWorkshareLoop does not exist yet, we might refactor this code once it exists. Comment Actions I second @Meinersbur comment. Reducing duplication is always better :)
Comment Actions Updated code, test case, fixed some issues createSections() now uses EmitOMPInlinedRegion(). The clang side updates will be pushed soon (D91054). This comment was removed by AMDChirag. Comment Actions @jdoerfert Ouch. Totally forgot about that, apologies, on it. Comment Actions It should replace a lot of logic in this, right? I would prefer to use the existing functionality right away.
This comment was removed by AMDChirag. This comment was removed by AMDChirag.
Comment Actions Added the use of createCanonicalLoop and createStaticWorkshareLoop. Comment Actions Currently cancellation clause is failing to work, within sections directive. Working on it. |