LGTM.

Two first comments. I will do a detailed review later.

LGTM. Thanks for the changes and the tests.

Might be good to have a test for fdebug-dump-parse-tree-no-sema.

@PeteSteinfeld I have reverted this patch since I could not find a quick fix.
https://reviews.llvm.org/rGb7ef804807855e607da3eba221c1fc59e27f778e

We have a buildbot CI failure due to this patch.
https://lab.llvm.org/buildbot/#/builders/33/builds/3463

@klausler We have a buildbot failing with the following error. https://lab.llvm.org/buildbot/#/builders/33/builds/3439

LGTM.

LGTM.

minor comments.

I made a trivial fix to get the builds to pass.
https://reviews.llvm.org/rG502f27e66fd9fe44cd45ec5acae3e18f15f2d8c6

LGTM. Thanks Peter for fixing this quickly.

The OpenMP for Flang team has consistently been asking for upstreaming the fir-dev branch. I made an initial attempt to upstream a portion in May last year (https://reviews.llvm.org/D79731) which was discarded since it
did not have any community support. The current situation presents a few difficulties for the OpenMP team.

1. The OpenMP dialect is in the llvm-project repo, while FIR codegen is developed in the fir-dev repo. So we have to first make a patch to llvm-project/mlir and then wait for it to be merged into the fir-dev repo (which can take from one to two weeks) and then make the relevant changes in the fir-dev repo. If FIR codegen was also upstream then this delay and committing to multiple repositories can be avoided.
2. Since the bridge code (parse-tree to FIR) and codegen is not available in llvm-project/flang, any commits that we make to fir-dev cannot be upstreamed. So all our changes are also increasing the diff between fir-dev and upstream llvm-project/flang. Left uncontrolled this might become an untameable monster and we might never be able to fully upstream fir-dev.
3. Since the OpenMP code which works with FIR is in fir-dev we cannot often show the context to MLIR core team. On at least one occasion this has become an issue while seeking help. If the code is upstream this will facilitate better discussions with the MLIR core team.

Failing a buildbot https://lab.llvm.org/buildbot/#/builders/33/builds/3150.

LGTM.

Addressed review comments.

In D98191#2611694, @tskeith wrote:

-fget-symbols-sources is not a debug option, it's intended for integrating with IDEs like vscode. So I think the original name is better. Unlike the "dump" options it actually is an action and not something that is intended to produce debug output on the way to doing something else.

Thanks for this patch. Couple of nits.

Just a quick comment. I will review in detail later.

Address review comments.
Updated syntax to specify the type along with the index.
Using BNF for parsing function documentation
Minor fixes, using llvm::interleaveComma.

@arnamoy10 can you submit this?

LGTM. Approving quickly since it fixes failures. Might need to do a recce and check that usage is consistent.

In D90697#2617519, @yhegde wrote:

This patch is updated with a few more test cases with max/min etc in omp-reduction07.f90 and also TODO comments. Hope they are acceptable.

Thanks to @clementval and @kiranchandramohan for accepting this PR and for the review comments. Thanks to @praveen for the kind co operation.

Added multi-block tests.

In D92327#2436879, @ftynse wrote:

I find the linear(%data_var : memref<i32> -> %linear_var) a bit confusing, we are using an equivalent of linear(%linear_var = %data_var : memref<i32>) in SCF and GPU dialects, and generally the %within_region = %outside_region : type pattern applies even more operations. -> seems to be predominantly used for results, which makes it confusing for me here.

Fixed failure in SCFToOpenMP.
Converted "->" to "=" for linear clause.

Ping @llitchev. Would you have time to take this forward?

LGTM. Please address all lint/clang-format errors. Please wait for @tskeith or @clementval to approve.

LGTM. Yes, using the symbol is the right way here.
Thanks @yhegde for separating this out into a new patch. Thanks also for you patience and all your work for semantic checks for OpenMP.

LGTM. Thanks for the patience.

If popping the end_sections helps you move forward, I am fine with it. Please go ahead.

Thanks @arnamoy10 for this patch.

Thanks @PeteSteinfeld.

I see a failure in the buildbot.
https://lab.llvm.org/buildbot/#/builders/33/builds/2845

The aarch64 flang buildbots are failing since this commit.
https://lab.llvm.org/buildbot/#/builders/32/builds/3480
The x86 buildbot is passing. https://lab.llvm.org/buildbot/#/builders/132

In D91920#2593420, @praveen wrote:

@kiranchandramohan The test case flang/test/Semantics/resolve102.f90 is failing with the error messages possibly due to the commit

https://github.com/llvm/llvm-project/commit/07de0846a5055015b55dc2b8faa2143f9902e549#diff-bc82e30d81709b26f2c8030300784e95003f1b968e1ed7842fa717858e497336

actual at 13: Procedure 'p' is recursively defined. Procedures in the cycle: 'p2', 'sub', 'p'
expect at 13: Procedure 'p' is recursively defined. Procedures in the cycle: 'p2', 'p', 'sub'
actual at 25: Procedure 'p' is recursively defined. Procedures in the cycle: 'p2', 'sub', 'p'
expect at 25: Procedure 'p' is recursively defined. Procedures in the cycle: 'p2', 'p', 'sub'

Can I push this patch now or only after the above failed case is fixed ?

There are a few comments not marked as done. Are these not done or have been just not marked done?

If you can add the autogen portion that will be great. It can be added either as part of this patch or a separate patch. But it need not be a blocker for this. I am approving this patch.

Did you miss the fir-ops tests for the new ops?

LGTM. This will be tested in the subsequent usage.

A few more questions.

Quick comments/questions before I have a final look.

Can you mark all comments which you believe are handled as done?

A few nits, comments and clarifications.

I guess for small format/clang-tidy changes reviews are not required.

LGTM.

LGTM. I have a Nit comment.

One question about blocks and a few nits.

A few questions and change requests.

Thank you for the review comments @kiranchandramohan . Can I split this patch one with threadprivate and cycle , dowhile etc with another patch ?

LGTM. Please address the clang-format comment before submission.

Thanks @yhegde for the changes. I believe this threadprivate check needs some more design and work. I don' t think we can clear the threadprivate symbols in module subprograms because it will fail to catch the error in test1 below. I think the threadprivate information should be captured in a symbol so that it is available in places where the module is used like in test2.

LGTM. Please consider Mehdi's comments before submitting.