[PATCH] D92732: [Flang][OpenMP 4.5] Add semantic check for OpenMP Do Loop Constructs
Yashaswini Hegde via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 27 11:08:50 PST 2021
yhegde added inline comments.
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:293
+ std::map<std::string, std::int64_t> labelNamesandLevels;
+ parser::CharBlock src{"cycle"};
+ if (collapseLevel) {
----------------
kiranchandramohan wrote:
> Is this variable used?
Will remove that. :)
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:294
+ parser::CharBlock src{"cycle"};
+ if (collapseLevel) {
+ if (const auto &loopConstruct{
----------------
kiranchandramohan wrote:
> 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
> ```
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
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:308-310
+ if (const auto *actionstmt{
+ std::get_if<parser::Statement<parser::ActionStmt>>(
+ &ec->u)}) {
----------------
kiranchandramohan wrote:
> 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.
parser::Walk on entire block was throwing error twice in few cases. Hence I called it with in a construct. Will check that.
================
Comment at: flang/test/Semantics/omp-do13.f90:1
+! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
----------------
kiranchandramohan wrote:
> Please add a test for an openmp do loop with collapse nested inside an openmp do loop with collapse.
ok.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92732/new/
https://reviews.llvm.org/D92732
More information about the llvm-commits
mailing list