LGTM.

In D97393#2666954, @Meinersbur wrote:

Could you add the following to the unittest?

Builder.restoreIP(EndIP);
Builder.CreateRetVoid();
OMPBuilder.finalize();
EXPECT_FALSE(verifyModule(*M, &errs()));

I checks whether the IR is internally consistent.

Updates as per review comments:

Updates requested in review:

• Avoid making temporary string
• Avoid British English in comment
• Be consistent and not mixing auto/named type in new tests

Add testing for chunk size in dynamic work sharing loop test.

LGTM

Nice test, thanks for adding that!

In D99198#2645402, @klausler wrote:

In D99198#2645322, @Leporacanthicus wrote:

I'm wondering if it's possible to construct a test-case for this "shuffle it around" situation, so that some newbie (like me ...) doesn't break it accidentally when making changes?

It gets exercised pretty thoroughly by the I/O tests in the FCVS Fortran '77 suite, especially by codes doing direct and sequential I/O, and the flang/unittests/Runtime/external-io.cpp test really shook it out on the read path (and still passes with this change now that it's used on the write path). But it would be nice to be able to make the code work harder by e.g. using an artificially small buffer size. I've done that in the past during development (but obviously not for console output, sorry) but I'm not sure how it could be made part of a unit testing framework.

I'm wondering if it's possible to construct a test-case for this "shuffle it around" situation, so that some newbie (like me ...) doesn't break it accidentally when making changes?

Fixes as per review comments

Fixed various review comments.

I'm going to upload the fixes once they compile, but it's getting late, so probably not until tomorrow morning.

In D97393#2585126, @Leporacanthicus wrote:

Please note that this is not intended as the final commit, but rather a basis for asking some advice on how to move forward.

The main stumbling point, which may be my lack of understanding of what it's supposed to do: the CanonicalLoopInfo assumes that the the cond block has a CmpInst as the first instruction. In the dynamic, the corresponding block [in my understanding] starts with a call instruction to fetch the "next" set of data to process. This causes the assertOK to fail, hence it is commented out on line 1300 in the patch.

Updated files based on selected review commets.
Fixed a few issues with the code-generation.
Added a basic test (similar to static workshare loop.

Please note that this is not intended as the final commit, but rather a basis for asking some advice on how to move forward.