Thanks.
LGTM.

Capitalized clauses in message.

Summary of changes:

We are slowly trying to replace the throwaway driver f18. That's actually a good progress.
Thank you for the progress @awarzynski @FarisRehman
I would wait for others to approve as this patch touches the core Fortran parts.

Looks good.

Ping.

Ok a clang version 8.0.1 based build passes.
It's seems compiler version related issue.
BTW, is there a way to mitigate or conform such compiler version/build related issues?
LGTM.

Thank you.
LGTM.

Agreed. @awarzynski. Thank you for clarification.

Thank you for working on this.
It build and tests successfully.
Please wait for other reviewers to accept.

This was a much needed change. Thank you for the refactoring.
One of the typical and most common problems was FindClause did not work as expected given that CheckAllowed was correctly defined for the clauses.

Thank you for the patch.
It was pending since many days.
The changes look good to me from the matching point w.r.t the linked patch.
I couldn't understand still the rational for *.cpp.inc to *.inc for both OMP and OACC, could you please point why was that needed?
Thank you.

In D93447#2498085, @awarzynski wrote:

Hi @sameeranjoshi , thank you for working on this!

Sadly with this patch one of our upstream builders has started failing: http://lab.llvm.org:8011/#/builders/33/builds/1866. More specifically,Semantics/omp-parallell01.f90 is failing.

All other builders are fine, which makes this weird. The only unique thing about the failing builder (flang-aarch64-ubuntu-clang) is that it uses clang-10. Could you take a look?

Edit: @kiranchandramohan just pointed out to me offline that this is probably fixed here: https://reviews.llvm.org/D94618.

Thank you for the patch.
I did get an email from the bot with a weird text Control flow escapes from \xcb\x1c\xda\xff\xff\nexpect at 16: Control flow escapes from PARALLEL\n'
I had a build of gcc 9.2 and the tried to check with the recent pull from main and all tests seem to work.
It was pointed[1] by @awarzynski that the issue was specifically due to clang-10.

Ideally in the first look the checks would fit better for ordered construct
inside a single function CheckDependOnOrderedDir so we have them at a
single place , unfortunately it's harder to implement it in above function
considering that the information to differentiate between a
OmpBlockDirective and OmpSimpleStandaloneDirective is unavailable.

Tweak comments as suggested.

These are great catches, thank you.
I was wondering since the merge of task_reduction why were the tests failing and did kept this on hold for a while since then.
Thanks again. :)

Summary of changes:

• Simplify the test as those checks were already covered.
• Update OMP.td to specifiy to add 5.0 standard for the clauses.

Summary of changes:

• Handle depend(source) and depend(sink:vec) separately.
• Handle one more restriction from standard[5.0][2.17.9] which was needed with this patch. Either depend(sink:vec) clauses or depend(source) clauses may appear on an ordered construct, but not both.

In D92735#2490685, @praveen wrote:

In D92735#2488356, @kiranchandramohan wrote:

Need an answer here to proceed with review.

@kiranchandramohan I am trying to resuse the function CheckBranchesIntoDoBody . Will update the patch regarding the same.

@sameeranjoshi Will you be merging the related changes https://reviews.llvm.org/D93447 that makes use of LabelEnforce ?

LGMT. Thanks for cleaning that up.

Looks good.

Summary of changes:

• Fix core dump in ordered construct due to missing tree node visit

for OpenMPSimpleStandaloneConstruct as ordered with a depend falls under it.

• Fix internal error in name resolution of depend clause.

Mostly nit comments.
Thanks and LGMT.

This patch was co-authored by @valentime. I forgot to update the commit message when pushing to main, apologies.

In D93105#2464268, @yhegde wrote:

In D93105#2461225, @kiranchandramohan wrote:

@yhegde Will the changes in this patch affect your reduction patch (https://reviews.llvm.org/D90697)?

Yes. Rt now I am directly getting the list of Designators . Now it will become a level of indirection with the list of OmpObjects.
I may be missing something but OpenMP 4.5 and 5.0 Reduction clause restrictions looks very similar , why this one level of indirection required?! Is it because functions names in Reduction clause can come from common blocks ?!

Thank you!
That was much needed to extract into a fixture, given a few upcoming reviews.

Remove unnecessary variable mayBeName.

Merge ResolveName into ResolveOmpName.

+1 for finding the issue.
Thanks for the efforts and LGMT.

Address review comments.

LGTM.
Please take care of the remaining nits comments before merging.
Thank you @tskeith for pointers.

Addresses one of the comments from
https://reviews.llvm.org/D88655#2356685

Please suggest.

Try to simplify comments.

Thanks for patch.
A few comments below.