[Flang][OpenMP 4.5] Add semantic check for OpenMP Do Loop Constructs

[yhegde]

Summary

Semantic check for OpenMP 4.5 - 2.7.1 Do Loop restrictions.
Implementation of Do loop iteration variable check , Do while loop check , Do loop threadprivate check,
Do loop cycle restrictions.

Files:
check-omp-structure.cpp
check-omp-structure.h
resolve-directives.cpp

TestCases:
omp-doloop02.f90
omp-doloop03.f90
omp-doloop04-pc.f90
omp-doloop04.f90

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

120

CheckDoControl and CheckDoVariable in flang/lib/Semantics/check-do-forall.cpp does the same check but are probably OFF or set as warning by default. Can the code in check-do-forall be reused?

154

Can we avoid having these global variables?

205

Are you handling cycle with the name of a loop?

497

Nit: Is this new line needed?

flang/lib/Semantics/check-omp-structure.h

192–194

Can we keep the declarations at the top or at the bottom. Please refer to style elsewhere.

flang/lib/Semantics/resolve-directives.cpp

800

FYI @sameeranjoshi has a patch which does something similarhttps://reviews.llvm.org/D92638.

[yhegde]

Test cases added for named do loops. Global variables removed.

TestCases:
omp-do04.f90
omp-do08.f90
omp-do09.f90
omp-do10.f90
omp-do11.f90
omp-do12.f90

[yhegde]

Implementation includes a check to "the ordered clause must be present on the loop construct if any ordered region ever binds to a loop region arising from the loop construct" - along with the previous Do loop iteration variable check , Do while loop check , Do loop threadprivate check, Do loop cycle restrictions.

Test case is omp-do06.f90.

[yhegde]

Rebased. Requesting the review comments.

[clementval]

flang/test/Semantics/omp-do06.f90

14 ↗

(On Diff #311494)

Is this a tab?

flang/test/Semantics/omp-do08.f90

71 ↗

(On Diff #311494)

tabs?

[clementval]

flang/test/Semantics/omp-do11.f90

1 ↗

(On Diff #311494)

Shouldn't you use the test_symbols.sh script for these tests?

[yhegde]

flang/test/Semantics/omp-do11.f90

1 ↗

(On Diff #311494)

I guess so. I have committed a few positive test cases with test_errors.sh earlier. I suppose I need to rework on them !!.

[yhegde]

This patch is updated with corrections to positive cases and indentations.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

109

If more than one loop is associated with the OpenMP do loop, should this check be performed on all the loops?

120

Is the answer here NO?

147–148

Why is collapse level used here but not in CheckLoopItrVariableIsInt?

205

I probably did not make myself clear here. What i was suggesting was that it is not enough to look at where the cycle exists but also have to consider where the cycle branches to. Consider the testcase below.

program test
  integer i, j, k
  !$omp do  collapse(2)
  foo: do i = 0, 10
         do j = 0, 10
           do k  = 0, 10
             if (k .lt. 1) cycle foo
             print *, i, j, k
           end do
         end do
  end do foo
  !$omp end do
end program

213

Is there an assumption here that if statement will be present when a cycle statement exists? Does the standard say that?

318

We do symbol related checks in resolve-directives right? Is there any particular reason this code should be here?

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

109

https://www.openmp.org/wp-content/uploads/openmp-4.5.pdf , page num 74, line num 5-6 says " If no collapse clause is present, the only loop that is associated with the loop construct is the one that immediately follows the directive."

So our understanding was if no collapse is present, only the iterations of outermost loop are distributed among threads.

gfrotran also throws error for only the outer loop.

120

I modified my code to suggested change. i.e I am doing similar kind of check like how it is done in check-do-forall.cpp

Will look at the rest of the review comments. Thank you.

[kiranchandramohan]

Please add @sameeranjoshi in all future patches.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

147–148

I think with or without collapse clause, do iteration variables in all loops should be checked for integer type.

205

Done. Thanks.

213

Cycles with out ifstmts are also handled.

318

All do iteration variable related checks have been implemented in check-omp-structure.cpp

[yhegde]

Addressed review comments.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

213

I think it might not work with else if, nested if etc.

Anyway, my point was that cycle statements can appear inside other fortran constructs, for e.g in a case statement like in the example below. So this portion of the code has to be generic and not limited to if statements.

program test
  integer i, j, k
  !$omp do collapse(3)
  foo: do i = 0, 10
    bar: do j = 0, 10
           do k = 0, 10
             select case (k)
               case (1)
                 cycle foo
               case (2)
                 cycle bar
               case default
                 cycle
             end select
           end do
         end do bar
  end do foo
  !$omp end do
end program

flang/lib/Semantics/check-omp-structure.h

203

Are you resetting this to null anywhere? Stale values might cause issues.

Instead of this method can you travel up the stack, find the do construct if any and check its clauses for the ordered clause?

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

213

Thanks for pointing out these issues. Can I have a class (OmpCycleBlockChecker) and parser::Walk to identify the cycles.? I have tried that for existing test cases (0mp-do08.f90 and omp-do13.f90 negative cases. and omp-do12.f90 and omp-do14.f90 - positive cases). I can extend it to other cases if it is acceptable.

flang/lib/Semantics/check-omp-structure.h

203

Thanks for the review comments. CheckIfDoOrderedClause () is called before pushingthecontext and associated clauses of the OMPD_do context are searched for the ordered clause.

[yhegde]

Code that are related to ordered clause and cycle stmts are modified to address the review comments.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

330

Is this variable used?

331

Will checks only happen for loops with collapse clause? What about those without collapse? Should it be handled elsewhere?

!$omp parallel
foo: do i = 0, 10
  !$omp do
  bar: do j = 0, 10
         cycle foo
       end do bar
  !$omp end do
  !$omp end parallel
end do foo

345–347

The concern with the approach is that you might have to call ompCycleBlockChecker for each individual construct. That is likely to be costly. Can parser::Walk happen on the entire block?

Do you have an estimate on how many additional visits will be there to a statement inside a Do loop due to this Walk? Will it be only 1 or more than that? If it is one additional visit it should be OK.

flang/test/Semantics/omp-do13.f90

1 ↗

(On Diff #319361)

Please add a test for an openmp do loop with collapse nested inside an openmp do loop with collapse.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

330

Will remove that. :)

331

Cases with OpenMP blocks in between are not handled . It requires different error message. But cases with out collapse - like the following case, works fine. Will add the test cases with out collapse. Thank you.
program test
!$omp parallel
foo: do i = 0, 10

!!$omp do
bar: do j = 0, 10
       cycle foo
end do bar
!!$omp end do

end do foo
!$omp end parallel
end program test

345–347

parser::Walk on entire block was throwing error twice in few cases. Hence I called it with in a construct. Will check that.

flang/test/Semantics/omp-do13.f90

1 ↗

(On Diff #319361)

ok.

[yhegde]

flang/test/Semantics/omp-do13.f90

1 ↗

(On Diff #319361)

Please add a test for an openmp do loop with collapse nested inside an openmp do loop with collapse.

Added a case like this . Hope this is what is suggested.

!$omp do collapse(2)

foo: do i = 0, 10
  foo1: do j = 0, 10
    !ERROR: CYCLE statement to non-innermost collapsed !$OMP DO loop
    cycle foo
    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
    !$omp do  collapse(1)
    foo2:  do k  = 0, 10
      print *, i, j, k
      end do foo2
   !$omp end do
   end do foo1
end do foo
!$omp end do

[yhegde]

The patch is updated with Cycle checks in IfConstruct and CaseConstruct with omp-do15.f90 ,omp-do16.f90 and omp-do17.f90 . The parser::Walk is out of the loop now and called only once. Cycles are mostly a concern when appear in do loops with collapse clause. Hence most of the test cases are with collapse.

[kiranchandramohan]

-> Threadprivate check is not working for the following case (when the declaration is in the parent scope).

program omp_do
  integer, save:: i, j, k
  !$omp threadprivate(i)
  !$omp threadprivate(j)
  call compute()
contains
  subroutine compute()
  !$omp  do collapse(2)
  do k = 1, 10
    do i = 1, 10
      print *, "Hello"
    end do
  end do
  !$omp end do
  end subroutine
end program omp_do

-> Do loop cycle restrictions should also include ordered besides collapse. I think this should be the same checks.

flang/lib/Semantics/check-omp-structure.cpp

66

We usually do not add the "!$" in the error message.

247

Can the "do" be capitalized for the loop and the directive?

264

Use DO loop instead of do-loop.

318

This clear will cause programs with multiple declarations to not work properly. See example below.

program omp_do
  integer, save:: i, j, k
  !$omp threadprivate(i)
  !$omp threadprivate(j)
  !$omp do collapse(2)
  do k = 1, 10
    do i = 1, 10
      print *, "Hello"
    end do
  end do
  !$omp end do
end program omp_do

331

My concern was because the cyclechecker is entered only if the collapse level is greater than 0. If there is no collapse then i guess it will return 0 and hence the cyclechecker might not be called.

std::int64_t collapseLevel{GetCollapseLevel(x)};
if (collapseLevel) {

flang/test/Semantics/omp-do13.f90

1 ↗

(On Diff #319361)

I mean't the following where you have the same error in both the inner and outer collapsed set of loops. Also add another one with parallel do collapse(2).

!$omp parallel
!$omp do collapse(2)
foo: do i = 0, 10
  foo1: do j = 0, 10
    cycle foo
    !$omp parallel
    !$omp do collapse(2)
    foo2:  do k  = 0, 10
      foo3:  do l  = 0, 10
      print *, i, j, k, l
      cycle foo2
      end do foo3
    end do foo2
    !$omp end do
    !$omp end parallel
   end do foo1
end do foo
!$omp end do
!$omp end parallel
end program

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

66

That is gfortran error message. Kindly suggest a better one. Will look into the rest of the review comments. Thank you.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

66

Would the following work?

"CYCLE statement to non-innermost associated loop of an OpenMP DO construct"

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

66

can that be "CYCLE statement to non-innermost associated loop of an OpenMP collapsed DO construct" ?

331

All negative test cases including those cases which have been suggested with parallel /parallel do collapse are all positive without collapse. means they give no error. please let me know if it is required to check for cycles in do loops with out collapse clause.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

66

Associated loops can be due to both collapse as well as ordered clauses. Hence I would prefer that you not add "collapsed" to the message.

331

Is your point that it should be handled by https://reviews.llvm.org/D92735 and not this patch? If so, I am fine if @praveen can confirm.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

331

Ok. Fine. I was under the impression that the cycles are the cause of error only with the collapse clause. I can add test cases with ordered clause also.

[praveen]

flang/lib/Semantics/check-omp-structure.cpp

331

Is your point that it should be handled by https://reviews.llvm.org/D92735 and not this patch? If so, I am fine if @praveen can confirm.

@kiranchandramohan The patch https://reviews.llvm.org/D92735 can be extended to check for cases such as the one below.

!$omp parallel
foo: do i = 0, 10
  !$omp do
  bar: do j = 0, 10
         cycle foo
       end do bar
  !$omp end do
  !$omp end parallel
end do foo

./omp-do-cycle.f90:15:18: error: invalid branch into an OpenMP structured block
           cycle foo
                 ^^^
./omp-do-cycle.f90:11:3: In the enclosing PARALLEL directive branched into
    foo: do i = 0, 10
    ^^^
./omp-do-cycle.f90:15:18: error: invalid branch leaving an OpenMP structured block
           cycle foo
                 ^^^
./omp-do-cycle.f90:11:3: Outside the enclosing DO directive
  foo: do i = 0, 10
  ^^^

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

331

@praveen I was hopeful that CheckNoBranching will catch the following error. Do you know why it does not catch in the current state? Is it just about calling CheckNoBranching in a few more places?

./omp-do-cycle.f90:15:18: error: invalid branch leaving an OpenMP structured block
           cycle foo
                 ^^^
./omp-do-cycle.f90:11:3: Outside the enclosing DO directive
  foo: do i = 0, 10

Since the source and destination of the branch are in the parallel region there should be no error for the parallel directive.

[praveen]

flang/lib/Semantics/check-omp-structure.cpp

331

@praveen I was hopeful that CheckNoBranching will catch the following error. Do you know why it does not catch in the current state? Is it just about calling CheckNoBranching in a few more places?

@kiranchandramohan Yes, as you suggested , CheckNoBranching should be able to catch this error , but since we are not handling "parser::CycleStmt" in the NoBranchingEnforce class it is not being checked.

Also CheckNoBranching is not being called for all the OpenMP structured blocks in the current state (It is checked as part of the patch for invalid branching checks)

The following is the output from handling the "parser::CycleStmt" in NoBranchingEnforce along with the changes in the patch https://reviews.llvm.org/D92735 for the below test case

!$omp parallel
foo: do i = 0, 10
  !$omp do
  bar: do j = 0, 10
         cycle foo
       end do bar
    !$omp end do
  !$omp end parallel
end do foo

error: CYCLE to construct 'foo' outside of DO construct is not allowed
           cycle foo
           ^^^^^^^^^
./d1.f90:13:9: Enclosing DO construct
  !$omp do
        ^^

For the cases such as this , two errors are identified for both the constructs

foo: do i = 0, 10
!$omp parallel
  !$omp do
  bar: do j = 0, 10
         cycle foo
       end do bar
       !$omp end do
  !$omp end parallel
end do foo

./omp-do-cycle.f90:15:12: error: CYCLE to construct 'foo' outside of PARALLEL construct is not allowed
           cycle foo
           ^^^^^^^^^
./omp-do-cycle.f90:11:9: Enclosing PARALLEL construct
  !$omp parallel
        ^^^^^^^^
./omp-do-cycle.f90:15:12: error: CYCLE to construct 'foo' outside of DO construct is not allowed
           cycle foo
           ^^^^^^^^^
./omp-do-cycle.f90:12:11: Enclosing DO construct
    !$omp do
          ^^

Please suggest if it is better to add the check as part of CheckNoBranching or in resolve-directives.cpp

Since the source and destination of the branch are in the parallel region there should be no error for the parallel directive.

Okay will add a check for the same

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

331

@praveen in my opinion handling "parser::CycleStmt" in NoBranchingEnforce and calling checkNoBranching is the right approach.

The above two errors match my expectation. If it matches yours also then please go ahead and make the change in https://reviews.llvm.org/D92735. If not then let me know the concern.

Thanks for your help here.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

318

@kiranchandramohan . Thanks for pointing this out. Can I have the ThreadPrivate related check in resolve-directives.cpp or is it required to do in check-omp-structure.cpp as it is openmp specific. ?

For the test case given (if implemented in resolve-directives.cpp)
program omp_do

integer, save:: i, j, k
!$omp threadprivate(i)
!$omp threadprivate(j)
!$omp do collapse(2)
do k = 1, 10
  do i = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do

end program omp_do

This is the error thrown
/tp.f90:10:8: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

and for omp-do04.f90

the errors are -
./omp-do04.f90:12:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./omp-do04.f90:14:8: error: Loop iteration variable j is not allowed in THREADPRIVATE.

do j = 1, 10
   ^

./omp-do04.f90:25:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./omp-do04.f90:26:8: error: Loop iteration variable j is not allowed in THREADPRIVATE.

do j = 1, 10
   ^

[praveen]

flang/lib/Semantics/check-omp-structure.cpp

331

@kiranchandramohan @yhegde Thanks . I have updated the patch https://reviews.llvm.org/D92735 with the changes for parser::CycleStmt in NoBranchingEnforce to handle cycle statements leaving the OpenMP structured blocks

[kiranchandramohan]

I think you are doing three patches worth of work in a single one. Ideally, we could have had, One patch for the cycle restrictions, One patch for threadprivate, and One patch for the rest.

flang/lib/Semantics/check-omp-structure.cpp

318

Yes, you can add the check in resolve-directives.cpp.

Why is there an error in line 26 of test omp-do04.f90? Since there is no collapse/ordered, should there be an error?

[yhegde]

In D92732#2537715, @kiranchandramohan wrote:

I think you are doing three patches worth of work in a single one. Ideally, we could have had, One patch for the cycle restrictions, One patch for threadprivate, and One patch for the rest.

Agreed. I suppose difficult to review also. but initially we thought it is good to have all do loop related checks committed together.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

318

Thank you @kiranchandramohan . I have updated the code and the test cases with ordered and collapsed clauses together. And the error for omp-do04.f90 is -

./omp-do04.f90:12:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./omp-do04.f90:14:8: error: Loop iteration variable j is not allowed in THREADPRIVATE.

do j = 1, 10
   ^

./omp-do04.f90:27:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

Now the thread private related code is moved to resolve-directives.cpp

[yhegde]

This patch address the review comments.
The cycle related checks are in OmpCycleChecker class.
The test cases omp-do08.f90 and then omp-do12.f90 to omp-do17.f90 are the test cases for cycle checks.

The Threadprivate check is moved to resolve-directives.cpp and omp-do04.f90 is the test case for that check.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

302–303

An else to the previous if is better.

flang/lib/Semantics/resolve-directives.cpp

753

Will this work if there are two omp do loops having threadprivate iteration variables?

[yhegde]

flang/lib/Semantics/resolve-directives.cpp

753

For the the following test case

program omp_do1

integer, save:: i, j, k, n
!$omp  threadprivate(i)
!$omp  do
!ERROR: Loop iteration variable i is not allowed in THREADPRIVATE.
do i = 1, 10
  !!$omp  threadprivate(j)
  !$omp  do
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do

!$omp  do
!ERROR: Loop iteration variable i is not allowed in THREADPRIVATE.
do i = 1, 10
  !!$omp  threadprivate(j)
  !$omp  do
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do

end program omp_do1

the error is

./kcmtp3.f90:6:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./kcmtp3.f90:8:12: error: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region

!$omp  do
       ^^

./kcmtp3.f90:17:6: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./kcmtp3.f90:19:12: error: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region

!$omp  do
       ^^

and if I uncomment !$omp threadprivate(j) , I get parse errors.

and the gfortran error is

kcmtp3.f90:6:14:

6 |   do i = 1, 10
  |              1

Error: !$OMP DO iteration variable must not be THREADPRIVATE at (1)
kcmtp3.f90:17:14:

17 |   do i = 1, 10
   |              1

Error: !$OMP DO iteration variable must not be THREADPRIVATE at (1)

[kiranchandramohan]

flang/lib/Semantics/resolve-directives.cpp

753

I got that slightly wrong. What i meant is that if we clear the symbols after visiting a block construct then we will miss the error on a following loop construct. For e.g.

integer, save:: i, j, k, n
!$omp  threadprivate(i)

!$omp parallel
print *, "parallel"
!$omp end parallel

!$omp  do
do i = 1, 10
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do

end program

[yhegde]

flang/lib/Semantics/resolve-directives.cpp

753

Probably I can call ClearThreadPrivateSymbols() in bool Pre(const parser::ModuleSubprogram &) ?

For the test case
program test
integer, save:: i, j, k, n
!$omp threadprivate(i)

!$omp parallel
print *, "parallel"
!$omp end parallel

!$omp do
do i = 1, 10

do j = 1, 10
  print *, "Hello"
end do

end do
!$omp end do

end program test

the error message is -
./tp4.f90:10:4: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

and for the test case

program omp_do2

!$omp threadprivate(k,i)
!$omp threadprivate(j)
!$omp  threadprivate(i)
call compute()
contains
subroutine compute()
!$omp  do ordered(2) collapse(2)
!ERROR: Loop iteration variable k is not allowed in THREADPRIVATE.
foo: do k = 1, 10
  do i = 1, 10
    print *, "Hello"
  end do
end do foo
!$omp end do
end subroutine

end program omp_do2

program omp_do3

!$omp  threadprivate(j)
!$omp parallel
print *, "parallel"
!$omp end parallel
!$omp  do
!ERROR: Loop iteration variable i is not allowed in THREADPRIVATE.
do j = 1, 10
  do i = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do

end program omp_do3

the error message is
./tp5.f90:10:11: error: Loop iteration variable k is not allowed in THREADPRIVATE.

foo: do k = 1, 10
        ^

./tp5.f90:11:8: error: Loop iteration variable i is not allowed in THREADPRIVATE.

do i = 1, 10
   ^

./tp5.f90:27:6: error: Loop iteration variable j is not allowed in THREADPRIVATE.

do j = 1, 10
   ^

[yhegde]

Addressed the review comments and rebased .

[kiranchandramohan]

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.

Would it be OK to move threadprivate checks to a separate patch? So that we can merge the other checks? We can even consider moving threadprivate out of the current statement of work since supporting this feature might be more effort than estimated. cc: @richard.barton.arm

test1

module md
integer :: i, j
!$omp  threadprivate(i)
contains
subroutine sb1
!$omp  do
do i = 1, 10
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do
end subroutine
end module

test2

module md
integer :: i
!$omp  threadprivate(i)
end module

program mn
use md 
!$omp  do
do i = 1, 10 
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do
end program

[yhegde]

In D92732#2551418, @kiranchandramohan wrote:

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.

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

Would it be OK to move threadprivate checks to a separate patch? So that we can merge the other checks? We can even consider moving threadprivate out of the current statement of work since supporting this feature might be more effort than estimated. cc: @richard.barton.arm

test1

module md
integer :: i, j
!$omp  threadprivate(i)
contains
subroutine sb1
!$omp  do
do i = 1, 10
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do
end subroutine
end module

test2

module md
integer :: i
!$omp  threadprivate(i)
end module

program mn
use md 
!$omp  do
do i = 1, 10 
  do j = 1, 10
    print *, "Hello"
  end do
end do
!$omp end do
end program

[kiranchandramohan]

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

Yes.
This patch can contain cycle, dowhile etc. Will try to approve it soon.
Another patch can contain the threadprivate checks.

[yhegde]

Based on Reviewer's comments , this patch is now updated with DoWhile , Cycles and other checks. Threadprivate related checks are removed.

[kiranchandramohan]

One question about blocks and a few nits.

flang/lib/Semantics/check-omp-structure.cpp

62

Nit this can just be

err = (it != labelNamesandLevels_.end() && it->second > 0);

78

Nit: cyclesource_ -> cycleSource_

247

Nit: Capitalize for the directive as well.
"The DO loop cannot be a DO WHILE with DO directive."

271–273

Is something missing here?
If "it" is block.begin() in the previous patch, how can it ever be block.end in this line unless the block is empty?

If nothing is missing, add a comment.

302

Nit: this condition is always true right? if so it can be reduced to an else statement (from else if).

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

271–273

Is something missing here?
If "it" is block.begin() in the previous patch, how can it ever be block.end in this line unless the block is empty?

I think If the block is empty then Fortran::parser::ExecutableConstruct has "no contained value". If block is not empty then DoConstruct, if exists in ExecutableConstruct, is fetched. Please let me know otherwise.

If nothing is missing, add a comment.

[yhegde]

Patch updated and rebased after addressing the review comments.

[kiranchandramohan]

LGTM. I have a Nit comment.

Did you create a separate PR for the threadprivate checks?

flang/lib/Semantics/check-omp-structure.cpp

271–273

Nit: Consider switching to the following.

loop = block.empty() ? nullptr : parser::Unwrap<parser::DoConstruct>(block.begin());

[yhegde]

In D92732#2562305, @kiranchandramohan wrote:

LGTM. I have a Nit comment.

Thank you @kiranchandramohan

Did you create a separate PR for the threadprivate checks?

Working on that.

[yhegde]

flang/lib/Semantics/check-omp-structure.cpp

271–273

Tried this. Didn't get expected results (errors). Block is a list type. Similar kind of implementation can be found in resolve-directives:806 , 1201.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

271–273

OK. Thanks. You can submit.

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   void Enter(const parser::OmpDependClause &);
   void Enter(const parser::OmpDistScheduleClause &);
   void Enter(const parser::OmpIfClause &);
   void Enter(const parser::OmpLinearClause &);
   void Enter(const parser::OmpMapClause &);
   void Enter(const parser::OmpProcBindClause &);
   void Enter(const parser::OmpReductionClause &);
   void Enter(const parser::OmpScheduleClause &);
+  void Enter(const parser::OpenMPThreadprivate &);
 
 private:
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
 
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,
       const parser::OmpScheduleModifierType::ModType &);
   void CheckAllowedMapTypes(const parser::OmpMapType::Type &,
       const std::list<parser::OmpMapType::Type> &);
   llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
 
   void CheckDependList(const parser::DataRef &);
   void CheckDependArraySection(
       const common::Indirection<parser::ArrayElement> &, const parser::Name &);
 
   void CheckIsVarPartOfAnotherVar(const parser::OmpObjectList &objList);
+
+  bool doWhileFlag{false};
+  Symbol *loopIv{nullptr};
+  std::list<Symbol *> threadPrivateSymbols;
+
+  void CheckIvForThreadPrivate(const parser::OpenMPLoopConstruct &x);
+  void CheckLoopItrVariableIsInt(const parser::OpenMPLoopConstruct &x);
+  void CheckDoWhile(const parser::OpenMPLoopConstruct &x);
+  void SetLoopIv(Symbol *symbol) { loopIv = symbol; }
+  const parser::Name GetLoopIndex(const parser::DoConstruct *x);
+  void CheckCycleConstraints(const parser::OpenMPLoopConstruct &x);
+  std::int64_t GetCollapseLevel(const parser::OpenMPLoopConstruct &x);
 };
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_

flang/lib/Semantics/check-omp-structure.cpp

 void OmpStructureChecker::Enter(const parser::OpenMPConstruct &) {
   // 2.8.1 TODO: Simd Construct with Ordered Construct Nesting check
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
   const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
 
   // check matching, End directive is optional
   if (const auto &endLoopDir{
           std::get<std::optional<parser::OmpEndLoopDirective>>(x.t)}) {
     const auto &endDir{
         std::get<parser::OmpLoopDirective>(endLoopDir.value().t)};
 
     CheckMatching<parser::OmpLoopDirective>(beginDir, endDir);
   }
 
   if (beginDir.v != llvm::omp::Directive::OMPD_do) {
     PushContextAndClauseSets(beginDir.source, beginDir.v);
   } else {
     // 2.7.1 do-clause -> private-clause |
     //                    firstprivate-clause |
     //                    lastprivate-clause |
     //                    linear-clause |
     //                    reduction-clause |
     //                    schedule-clause |
     //                    collapse-clause |
     //                    ordered-clause
 
     // nesting check
     HasInvalidWorksharingNesting(beginDir.source,
         {llvm::omp::Directive::OMPD_do, llvm::omp::Directive::OMPD_sections,
             llvm::omp::Directive::OMPD_single,
             llvm::omp::Directive::OMPD_workshare,
             llvm::omp::Directive::OMPD_task,
             llvm::omp::Directive::OMPD_taskloop,
             llvm::omp::Directive::OMPD_critical,
             llvm::omp::Directive::OMPD_ordered,
             llvm::omp::Directive::OMPD_atomic,
             llvm::omp::Directive::OMPD_master});
     PushContextAndClauseSets(beginDir.source, llvm::omp::Directive::OMPD_do);
   }
+  CheckDoWhile(x);
+  CheckLoopItrVariableIsInt(x);
+  CheckIvForThreadPrivate(x);
+  CheckCycleConstraints(x);
+}
+
+void OmpStructureChecker::CheckDoWhile(const parser::OpenMPLoopConstruct &x) {
+  const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+  const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
+  doWhileFlag = false;
+  if (beginDir.v == llvm::omp::Directive::OMPD_do) {
+    if (const auto &doConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+      if (doConstruct.value().IsDoWhile()) {
+        doWhileFlag = true;
+        auto &doStmt{std::get<parser::Statement<parser::NonLabelDoStmt>>(
+            doConstruct.value().t)};
+        context_.Say(doStmt.source,
+            "The do loop cannot be a DO WHILE with do directive."_err_en_US);
+      }
+    }
+  }
+}
+
+void OmpStructureChecker::CheckLoopItrVariableIsInt(
+    const parser::OpenMPLoopConstruct &x) {
+  if (!doWhileFlag) {
+    if (const auto &loopConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+      for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
+        // go through all the nested do-loops and to get index variables
+        const parser::Name &itrVal{GetLoopIndex(loop)};
+        if (itrVal.symbol) {
+          auto type{itrVal.symbol->GetType()};
+          if (auto *tySpec{type->AsIntrinsic()}) {
+            if (tySpec->category() != Fortran::common::TypeCategory::Integer) {
+              context_.Say(itrVal.source,
+                  "The do-loop iteration"
+                  " variable must be of the type integer."_err_en_US,
+                  itrVal.ToString());
+            }
+          }
+        }
+        const auto &block{std::get<parser::Block>(loop->t)};
+        const auto it{block.begin()};
+        loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+                                 : nullptr;
+      }
+    }
+  }
+}
+
+void OmpStructureChecker::CheckIvForThreadPrivate(
+    const parser::OpenMPLoopConstruct &x) {
+
+  std::int64_t collapseLevel{GetCollapseLevel(x)};
+  if(!collapseLevel) collapseLevel++;
+
+  if (!doWhileFlag && loopIv) {
+    if (const auto &loopConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+
+      if (!threadPrivateSymbols.empty()) {
+        for (const parser::DoConstruct *loop{&*loopConstruct}; loop && collapseLevel; --collapseLevel) {
+          // go through all the nested do-loops and to get index variables
+          const parser::Name &itrVal{GetLoopIndex(loop)};
+
+          for (auto tps : threadPrivateSymbols) {
+            if (itrVal.symbol->name() == tps->name()) {
+              context_.Say(itrVal.source,
+                  "Loop iteration variable %s is not allowed in threadprivate."_err_en_US,
+                  itrVal.ToString());
+            }
+          }
+          const auto &block{std::get<parser::Block>(loop->t)};
+          const auto it{block.begin()};
+          loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+                                   : nullptr;
+        }
+      }
+    }
+  }
+}
+
+const parser::Name OmpStructureChecker::GetLoopIndex(
+    const parser::DoConstruct *x) {
+  using Bounds = parser::LoopControl::Bounds;
+  return std::get<Bounds>(x->GetLoopControl()->u).name.thing;
+}
+
+std::int64_t OmpStructureChecker::GetCollapseLevel(
+    const parser::OpenMPLoopConstruct &x) {
+  const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+  const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};
+
+  std::int64_t collapseLevel{0};
+
+  for (const auto &clause : clauseList.v) {
+    if (const auto *collapseClause{
+            std::get_if<parser::OmpClause::Collapse>(&clause.u)}) {
+      if (const auto v{GetIntValue(collapseClause->v)}) {
+        collapseLevel = *v;
+        break;
+      }
+    }
+  }
+  return collapseLevel;
+}
+
+void OmpStructureChecker::CheckCycleConstraints(
+    const parser::OpenMPLoopConstruct &x) {
+  std::int64_t collapseLevel{GetCollapseLevel(x)};
+  std::int64_t cycleLevel{collapseLevel-1};
+  const parser::DoConstruct *docns{nullptr};
+  if (collapseLevel) {
+    if (const auto &loopConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+      for (const parser::DoConstruct *loop{&*loopConstruct}; loop;
+           --cycleLevel) {
+        docns = loop;
+        const parser::Block &blk{std::get<parser::Block>(docns->t)};
+        for (auto iit{blk.begin()}; iit != blk.end(); ++iit) {
+          if (const auto *ifstmt{parser::Unwrap<parser::IfStmt>(*iit)}) {
+            const auto &actionstmt{std::get<1>(ifstmt->t)};
+            if (parser::Unwrap<parser::CycleStmt>(actionstmt.statement.u)) {
+
+              if (cycleLevel > 0) {
+                context_.Say(actionstmt.source,
+                    "CYCLE statement to non-innermost collapsed !$OMP DO loop"_err_en_US);
+              }
+            }
+          }
+        }
+        const parser::Block &block{std::get<parser::Block>(loop->t)};
+        const auto it{block.begin()};
+        loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+                                 : nullptr;
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) {
   const auto &dir{std::get<parser::OmpLoopDirective>(x.t)};
   ResetPartialContext(dir.source);
   switch (dir.v) {
   // 2.7.1 end-do -> END DO [nowait-clause]
   // 2.8.3 end-do-simd -> END DO SIMD [nowait-clause]
   case llvm::omp::Directive::OMPD_do:
   case llvm::omp::Directive::OMPD_do_simd:
     SetClauseSets(dir.v);
     break;
   default:
     // no clauses are allowed
     break;
   }
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   const auto &beginBlockDir{std::get<parser::OmpBeginBlockDirective>(x.t)};
   const auto &endBlockDir{std::get<parser::OmpEndBlockDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
   const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir.t)};
   const parser::Block &block{std::get<parser::Block>(x.t)};
 
   CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
 
   PushContextAndClauseSets(beginDir.source, beginDir.v);
 
   switch (beginDir.v) {
   case llvm::omp::OMPD_parallel:
     CheckNoBranching(block, llvm::omp::OMPD_parallel, beginDir.source);
     break;
   default:
     break;
   }
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
   dirContext_.pop_back();
 }
 

 void OmpStructureChecker::Leave(const parser::OpenMPSectionsConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndSectionsDirective &x) {
   const auto &dir{std::get<parser::OmpSectionsDirective>(x.t)};
   ResetPartialContext(dir.source);
   switch (dir.v) {
     // 2.7.2 end-sections -> END SECTIONS [nowait-clause]
   case llvm::omp::Directive::OMPD_sections:
     PushContextAndClauseSets(
         dir.source, llvm::omp::Directive::OMPD_end_sections);
     break;
   default:
     // no clauses are allowed
     break;
   }
 }
+void OmpStructureChecker::Enter(const parser::OpenMPThreadprivate &x) {
 
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContextAndClauseSets(
+      dir.source, llvm::omp::Directive::OMPD_threadprivate);
+
+  threadPrivateSymbols.clear();
+  const auto &list{std::get<parser::OmpObjectList>(x.t)};
+
+  for (const auto &ompObject : list.v) {
+    std::visit(common::visitors{
+                   [&](const parser::Designator &designator) {
+                     const auto *dataRef{
+                         std::get_if<parser::DataRef>(&designator.u)};
+                     if (dataRef) {
+                       if (const parser::Name *
+                           name{std::get_if<parser::Name>(&dataRef->u)}) {
+                         SetLoopIv(name->symbol);
+                         if (std::find(threadPrivateSymbols.begin(),
+                                 threadPrivateSymbols.end(),
+                                 name->symbol) == threadPrivateSymbols.end()) {
+                           threadPrivateSymbols.push_back(name->symbol);
+                         }
+                       }
+                     }
+                   },
+                   [&](const parser::Name &name) {},
+               },
+        ompObject.u);
+  }
+}
 void OmpStructureChecker::Enter(const parser::OpenMPDeclareSimdConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_declare_simd);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPDeclareSimdConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);
   const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
if (auto *clause{FindClause(llvm::omp::Clause::OMPC_ordered)}) {
       // TODO: ordered region binding check (requires nesting implementation)
     }
   } // doSet
 
   // 2.8.1 Simd Construct Restriction
   if (llvm::omp::simdSet.test(GetContext().directive)) {
     if (auto *clause{FindClause(llvm::omp::Clause::OMPC_simdlen)}) {
       if (auto *clause2{FindClause(llvm::omp::Clause::OMPC_safelen)}) {
+
         const auto &simdlenClause{
             std::get<parser::OmpClause::Simdlen>(clause->u)};
         const auto &safelenClause{
             std::get<parser::OmpClause::Safelen>(clause2->u)};
         // simdlen and safelen both have parameters
         if (const auto simdlenValue{GetIntValue(simdlenClause.v)}) {
           if (const auto safelenValue{GetIntValue(safelenClause.v)}) {
             if (*safelenValue > 0 && *simdlenValue > *safelenValue) {

flang/lib/Semantics/resolve-directives.cpp

private:
   Symbol *DeclareOrMarkOtherAccessEntity(Symbol &, Symbol::Flag);
   void CheckMultipleAppearances(
       const parser::Name &, const Symbol &, Symbol::Flag);
 
   void CheckDataCopyingClause(
       const parser::Name &, const Symbol &, Symbol::Flag);
 
   void CheckAssocLoopLevel(std::int64_t level, const parser::OmpClause *clause);
+  bool doWhileFlag{false};
 };
 
 template <typename T>
 bool DirectiveAttributeVisitor<T>::HasDataSharingAttributeObject(
     const Symbol &object) {
   auto it{dataSharingAttributeObjects_.find(object)};
   return it != dataSharingAttributeObjects_.end();
 }
for (const auto *allocName : allocateNames_) {
             allocName->ToString());
       }
     }
     break;
   }
   default:
     break;
   }
   PopContext();
 }
 
 bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
   const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
   const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};
   switch (beginDir.v) {
   case llvm::omp::Directive::OMPD_distribute:
bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
   case llvm::omp::Directive::OMPD_teams_distribute_simd:
     PushContext(beginDir.source, beginDir.v);
     break;
   default:
     break;
   }
   ClearDataSharingAttributeObjects();
   SetContextAssociatedLoopLevel(GetAssociatedLoopLevelFromClauses(clauseList));
+
+  if (beginDir.v == llvm::omp::Directive::OMPD_do) {
+    if (const auto &doConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+      if (doConstruct.value().IsDoWhile()) {
+        doWhileFlag = true;
+      }
+    }
+  }
+  if (!doWhileFlag) {
     PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
+  }
   return true;
 }
 
 void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
     const parser::Name &iv) {
   auto targetIt{dirContext_.rbegin()};
   for (;; ++targetIt) {
     if (targetIt == dirContext_.rend()) {
void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
   }
 }
 
 // 2.15.1.1 Data-sharing Attribute Rules - Predetermined
 //   - A loop iteration variable for a sequential loop in a parallel
 //     or task generating construct is private in the innermost such
 //     construct that encloses the loop
 bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
-  if (!dirContext_.empty() && GetContext().withinConstruct) {
+  if (!dirContext_.empty() && GetContext().withinConstruct && !doWhileFlag) {
     if (const auto &iv{GetLoopIndex(x)}; iv.symbol) {
       if (!iv.symbol->test(Symbol::Flag::OmpPreDetermined)) {
         ResolveSeqLoopIndexInParallelOrTaskConstruct(iv);
       } else {
         // TODO: conflict checks with explicitly determined DSA
       }
     }
   }

flang/test/Semantics/omp-doloop-positivecases.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop Constructs
+
+!DEF: /omp_do MainProgram
+program omp_do
+  !DEF: /omp_do/i ObjectEntity INTEGER(4)
+  !DEF: /omp_do/j ObjectEntity INTEGER(4)
+  !DEF: /omp_do/k ObjectEntity INTEGER(4)
+  integer i, j, k
+  !$omp do
+  !DEF: /omp_do/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=1,10
+    !REF: /omp_do/j
+    do j=1,10
+      !REF: /omp_do/Block1/i
+      !REF: /omp_do/j
+      print *, "it", i, j
+    end do
+  end do
+  !$omp end do
+end program omp_do
+
+!DEF: /omp_do1 MainProgram
+program omp_do1
+  !DEF: /omp_do/i ObjectEntity INTEGER(4)
+  !DEF: /omp_do/j ObjectEntity INTEGER(4)
+  !DEF: /omp_do/k (OmpThreadprivate) ObjectEntity INTEGER(4)
+  integer i, j, k, n
+  !$omp threadprivate (k,n)
+  !$omp do
+  !DEF: /omp_do/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=1,10
+    !REF: /omp_do/j
+    do j=1,10
+      print *, "Hello"
+    end do
+  end do
+  !$omp end do
+end program omp_do1
+
+!DEF: /omp_do3 MainProgram
+program omp_do3
+  !DEF: /omp_do/i ObjectEntity INTEGER(4)
+  !DEF: /omp_do/k ObjectEntity INTEGER(4)
+  integer :: i = 0, k
+  !$omp do
+  !DEF: /omp_do/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=1,10
+    !REF: /omp_do/Block1/i
+    print *, "it", i
+  end do
+  !$omp end do
+end program omp_do3

flang/test/Semantics/omp-doloop01.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop constructs.
+
+program omp_do
+  real i, j, k
+  !$omp do
+  !ERROR: The do-loop iteration variable must be of the type integer.
+  do i = 1, 10
+    !ERROR: The do-loop iteration variable must be of the type integer.
+    do j = 1, 10
+      print *, "it", i, j
+    end do
+  end do
+  !$omp end do
+end program omp_do
+

flang/test/Semantics/omp-doloop02.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop Constructs
+
+program omp_do
+  integer, save:: i, j, k,n
+  !$omp  threadprivate(k,j,i)
+  !$omp  do collapse(2)
+  !ERROR: Loop iteration variable i is not allowed in threadprivate.
+  do i = 1, 10
+    !ERROR: Loop iteration variable j is not allowed in threadprivate.
+    do j = 1, 10
+      print *, "Hello"
+    end do
+  end do
+  !$omp end do
+end program omp_do
+
+program omp_do1
+  !$omp  threadprivate(k,j,i)
+  !$omp  do
+  !ERROR: Loop iteration variable i is not allowed in threadprivate.
+  do i = 1, 10
+    do j = 1, 10
+      print *, "Hello"
+    end do
+  end do
+  !$omp end do
+end program omp_do1

flang/test/Semantics/omp-doloop03.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop constructs.
+
+program omp_do
+  integer ::  i = 0,k
+  !$omp do
+  !ERROR: The do loop cannot be a DO WHILE with do directive.
+  do while (i <= 10)
+    print *, "it",i
+    i = i+1
+  end do
+  !$omp end do
+
+  !$omp do
+  !ERROR: The do loop cannot be a DO WHILE with do directive.
+  do while (i <= 10)
+    do while (j <= 10)
+      print *, "it",k
+      j = j+1
+    end do
+    i = i+1
+  end do
+  !$omp end do
+end program omp_do
+

flang/test/Semantics/omp-doloop04-pc.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop constructs.
+
+!DEF: /omp_cycle MainProgram
+program omp_cycle
+  !$omp do  collapse(1)
+  !DEF: /omp_cycle/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=0,10
+    !REF: /omp_cycle/Block1/i
+    if (i<1) cycle
+    !DEF: /omp_cycle/j (Implicit) ObjectEntity INTEGER(4)
+    do j=0,10
+      !DEF: /omp_cycle/k (Implicit) ObjectEntity INTEGER(4)
+      do k=0,10
+        !REF: /omp_cycle/Block1/i
+        !REF: /omp_cycle/j
+        !REF: /omp_cycle/k
+        print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+
+  !$omp do  collapse(1)
+  !DEF: /omp_cycle/Block2/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=0,10
+    !REF: /omp_cycle/j
+    do j=0,10
+      !REF: /omp_cycle/Block2/i
+      if (i<1) cycle
+      !REF: /omp_cycle/k
+      do k=0,10
+        !REF: /omp_cycle/Block2/i
+        !REF: /omp_cycle/j
+        !REF: /omp_cycle/k
+        print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+
+  !$omp do  collapse(3)
+  !DEF: /omp_cycle/Block3/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+  do i=0,10
+    !DEF: /omp_cycle/Block3/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+    do j=0,10
+      !DEF: /omp_cycle/Block3/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+      do k=0,10
+        !REF: /omp_cycle/Block3/i
+        if (i<1) cycle
+        !REF: /omp_cycle/Block3/i
+        !REF: /omp_cycle/Block3/j
+        !REF: /omp_cycle/Block3/k
+        print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+end program omp_cycle

flang/test/Semantics/omp-doloop04.f90

+!RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
+! 2.7.1 Do Loop constructs.
+
+program omp
+  integer i, j, k
+
+  !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
+  !$omp do  collapse(3)
+  do i = 0, 10
+    !ERROR: CYCLE statement to non-innermost collapsed !$OMP DO loop
+    if (i .lt. 1) cycle
+    do j = 0, 10
+      do k  = 0, 10
+       print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+
+
+  !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
+  !$omp do  collapse(3)
+  do i = 0, 10
+    do j = 0, 10
+      !ERROR: CYCLE statement to non-innermost collapsed !$OMP DO loop
+      if (i .lt. 1) cycle
+      do k  = 0, 10
+       print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+
+  !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
+  !$omp do  collapse(2)
+  do i = 0, 10
+    !ERROR: CYCLE statement to non-innermost collapsed !$OMP DO loop
+    if (i .lt. 1) cycle
+    do j = 0, 10
+      do k  = 0, 10
+       print *, i, j, k
+      end do
+    end do
+  end do
+  !$omp end do
+
+end program omp