[flang][OpenMP] Parser support for the unroll construct (5.1)

[abidmalikwaterloo]

Summary

added parser support for the unroll construct

[kiranchandramohan]

There is a clang-format issue with flang/lib/Parser/openmp-parsers.cpp and flang/lib/Parser/unparse.cpp.

You have missed support for the full and partial clauses. For adding support, you will have to make some changes to parse these clauses in flang/lib/Parser/openmp-parsers.cpp and llvm/include/llvm/Frontend/OpenMP/OMP.td.

Please also add tests with these clauses.

flang/test/Parser/omp-unroll.f90

7

Nit: y is unused.

13

Nit: no need for semicolon

15

Nit: no need for semicolon

[abidmalikwaterloo]

Added support for the clauses . Added new tests

[kiranchandramohan]

Requesting a change in the OMP.td file. Also, there are clang-format issues.

 clang-format
changed files:
    flang/lib/Parser/openmp-parsers.cpp
    flang/lib/Parser/unparse.cpp

flang/lib/Parser/openmp-parsers.cpp

296

Nit: accidental change?

flang/test/Parser/omp-unroll.f90

9

Nit: check for the partial unroll value as well.

llvm/include/llvm/Frontend/OpenMP/OMP.td

76

The standard says that this is a Compile Time Constant so i guess ScalarIntConstantExpr is the right flangClass here.

where unroll-factor is a positive integer expression that is a compile-time constant.

[abidmalikwaterloo]

Ahh! I thought I fixed the clang-format issues. will look into it.

[abidmalikwaterloo]

Updated the test and took care of the reviewer's comments

[abidmalikwaterloo]

Ping!

[kiranchandramohan]

The comment about using scalarIntConstantExpr instead of scalarIntExpr is not yet addressed.

flang/lib/Parser/openmp-parsers.cpp

256

I think scalarIntConstantExpr is the correct one to use here.

llvm/include/llvm/Frontend/OpenMP/OMP.td

76

This comment is not addressed.

[abidmalikwaterloo]

replace scalarIntExpr with scalarIntConstantExpr.

[kiranchandramohan]

LGTM. Thanks for your patience. Please update the expectation in the test for the CI and tests to pass before submitting.

flang/test/Parser/omp-unroll.f90

21

The expected value here needs an update.

[abidmalikwaterloo]

Updated the test case omp-unroll.f90

flang/lib/Parser/openmp-parsers.cpp

"DEVICE" >> construct<OmpClause>(construct<OmpClause::Device>(
                     parenthesized(Parser<OmpDeviceClause>{}))) ||
     "DIST_SCHEDULE" >>
         construct<OmpClause>(construct<OmpClause::DistSchedule>(
             parenthesized("STATIC" >> maybe("," >> scalarIntExpr)))) ||
     "DYNAMIC_ALLOCATORS" >>
         construct<OmpClause>(construct<OmpClause::DynamicAllocators>()) ||
     "FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
                    parenthesized(scalarLogicalExpr))) ||
+    "FULL" >>  construct<OmpClause>(construct<OmpClause::Full>()) ||
     "FIRSTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Firstprivate>(
                           parenthesized(Parser<OmpObjectList>{}))) ||
     "FROM" >> construct<OmpClause>(construct<OmpClause::From>(
                   parenthesized(Parser<OmpObjectList>{}))) ||
     "GRAINSIZE" >> construct<OmpClause>(construct<OmpClause::Grainsize>(
                        parenthesized(scalarIntExpr))) ||
     "HINT" >> construct<OmpClause>(
                   construct<OmpClause::Hint>(parenthesized(constantExpr))) ||
TYPE_PARSER(
     "NUM_TASKS" >> construct<OmpClause>(construct<OmpClause::NumTasks>(
                        parenthesized(scalarIntExpr))) ||
     "NUM_TEAMS" >> construct<OmpClause>(construct<OmpClause::NumTeams>(
                        parenthesized(scalarIntExpr))) ||
     "NUM_THREADS" >> construct<OmpClause>(construct<OmpClause::NumThreads>(
                          parenthesized(scalarIntExpr))) ||
     "ORDERED" >> construct<OmpClause>(construct<OmpClause::Ordered>(
                      maybe(parenthesized(scalarIntConstantExpr)))) ||
+    "PARTIAL" >> construct<OmpClause>(construct<OmpClause::Partial>(
+                     maybe(parenthesized(scalarIntConstantExpr)))) ||                 
     "PRIORITY" >> construct<OmpClause>(construct<OmpClause::Priority>(
                       parenthesized(scalarIntExpr))) ||
     "PRIVATE" >> construct<OmpClause>(construct<OmpClause::Private>(
                      parenthesized(Parser<OmpObjectList>{}))) ||
     "PROC_BIND" >> construct<OmpClause>(construct<OmpClause::ProcBind>(
                        parenthesized(Parser<OmpProcBindClause>{}))) ||
     "REDUCTION" >> construct<OmpClause>(construct<OmpClause::Reduction>(
                        parenthesized(Parser<OmpReductionClause>{}))) ||
"TO" >> construct<OmpClause>(construct<OmpClause::To>(
                 parenthesized(Parser<OmpObjectList>{}))) ||
     "USE_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::UseDevicePtr>(
                             parenthesized(nonemptyList(name)))) ||
     "UNIFIED_ADDRESS" >>
         construct<OmpClause>(construct<OmpClause::UnifiedAddress>()) ||
     "UNIFIED_SHARED_MEMORY" >>
         construct<OmpClause>(construct<OmpClause::UnifiedSharedMemory>()) ||
     "UNIFORM" >> construct<OmpClause>(construct<OmpClause::Uniform>(
                      parenthesized(nonemptyList(name)))) ||
     "UNTIED" >> construct<OmpClause>(construct<OmpClause::Untied>()))
 
 // [Clause, [Clause], ...]
 TYPE_PARSER(sourced(construct<OmpClauseList>(
     many(maybe(","_tok) >> sourced(Parser<OmpClause>{})))))
 
 // 2.1 (variable | /common-block | array-sections)
 TYPE_PARSER(construct<OmpObjectList>(nonemptyList(Parser<OmpObject>{})))
TYPE_PARSER(sourced(construct<OmpLoopDirective>(first(
     "TASKLOOP SIMD" >> pure(llvm::omp::Directive::OMPD_taskloop_simd),
     "TASKLOOP" >> pure(llvm::omp::Directive::OMPD_taskloop),
     "TEAMS DISTRIBUTE PARALLEL DO SIMD" >>
         pure(llvm::omp::Directive::OMPD_teams_distribute_parallel_do_simd),
     "TEAMS DISTRIBUTE PARALLEL DO" >>
         pure(llvm::omp::Directive::OMPD_teams_distribute_parallel_do),
     "TEAMS DISTRIBUTE SIMD" >>
         pure(llvm::omp::Directive::OMPD_teams_distribute_simd),
-    "TEAMS DISTRIBUTE" >> pure(llvm::omp::Directive::OMPD_teams_distribute)))))
+    "TEAMS DISTRIBUTE" >> pure(llvm::omp::Directive::OMPD_teams_distribute),
+    "UNROLL">> pure(llvm::omp::Directive::OMPD_unroll)))))
 
 TYPE_PARSER(sourced(construct<OmpBeginLoopDirective>(
     sourced(Parser<OmpLoopDirective>{}), Parser<OmpClauseList>{})))
 
 // 2.14.1 construct-type-clause -> PARALLEL | SECTIONS | DO | TASKGROUP
 TYPE_PARSER(sourced(construct<OmpCancelType>(
     first("PARALLEL" >> pure(OmpCancelType::Type::Parallel),
         "SECTIONS" >> pure(OmpCancelType::Type::Sections),

flang/lib/Parser/unparse.cpp

case llvm::omp::Directive::OMPD_teams_distribute_parallel_do:
       Word("TEAMS DISTRIBUTE PARALLEL DO ");
       break;
     case llvm::omp::Directive::OMPD_teams_distribute_parallel_do_simd:
       Word("TEAMS DISTRIBUTE PARALLEL DO SIMD ");
       break;
     case llvm::omp::Directive::OMPD_teams_distribute_simd:
       Word("TEAMS DISTRIBUTE SIMD ");
       break;
+    case llvm::omp::Directive::OMPD_unroll:
+      Word("UNROLL ");  
+      break;
     default:
       break;
     }
   }
   void Unparse(const OmpObjectList &x) { Walk(x.v, ","); }
   void Unparse(const OmpSimpleStandaloneDirective &x) {
     switch (x.v) {
     case llvm::omp::Directive::OMPD_barrier:

flang/lib/Semantics/resolve-directives.cpp

bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
   case llvm::omp::Directive::OMPD_target_teams_distribute_simd:
   case llvm::omp::Directive::OMPD_target_simd:
   case llvm::omp::Directive::OMPD_taskloop:
   case llvm::omp::Directive::OMPD_taskloop_simd:
   case llvm::omp::Directive::OMPD_teams_distribute:
   case llvm::omp::Directive::OMPD_teams_distribute_parallel_do:
   case llvm::omp::Directive::OMPD_teams_distribute_parallel_do_simd:
   case llvm::omp::Directive::OMPD_teams_distribute_simd:
+  case llvm::omp::Directive::OMPD_unroll:
     PushContext(beginDir.source, beginDir.v);
     break;
   default:
     break;
   }
   ClearDataSharingAttributeObjects();
   SetContextAssociatedLoopLevel(GetAssociatedLoopLevelFromClauses(clauseList));
 

flang/test/Parser/omp-unroll-full.f90

+! RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck --ignore-case %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+subroutine openmp_parse_unroll(x)
+
+  integer, intent(inout)::x
+
+!CHECK: !$omp unroll full
+!$omp  unroll full
+!CHECK: do
+  do x = 1, 100
+    call F1()
+!CHECK: end do
+  end do
+!CHECK: !$omp end unroll
+!$omp end unroll
+
+!PARSE-TREE: OpenMPConstruct -> OpenMPLoopConstruct
+!PARSE-TREE: OmpBeginLoopDirective
+!PARSE-TREE: OmpLoopDirective -> llvm::omp::Directive = unroll
+!PARSE-TREE: OmpClauseList -> OmpClause -> Full
+END subroutine openmp_parse_unroll

flang/test/Parser/omp-unroll.f90

+! RUN: %flang_fc1 -fdebug-unparse-no-sema -fopenmp %s | FileCheck --ignore-case %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+subroutine openmp_parse_unroll(x)
+
+  integer, intent(inout)::x
+
+!CHECK: !$omp unroll partial(3)
+!$omp  unroll partial(3)
+!CHECK: do
+  do x = 1, 100
+    call F1()
+!CHECK: end do
+  end do
+!CHECK: !$omp end unroll
+!$omp end unroll
+
+!PARSE-TREE: OpenMPConstruct -> OpenMPLoopConstruct
+!PARSE-TREE: OmpBeginLoopDirective
+!PARSE-TREE: OmpLoopDirective -> llvm::omp::Directive = unroll
+!PARSE-TREE: OmpClauseList -> OmpClause -> Partial -> Scalar -> Integer -> Expr = '3_4'
+!PARSE-TREE: LiteralConstant -> IntLiteralConstant = '3'
+
+END subroutine openmp_parse_unroll

llvm/include/llvm/Frontend/OpenMP/OMP.td

def OMPC_Default : Clause<"default"> {
   let clangClass = "OMPDefaultClause";
   let flangClass = "OmpDefaultClause";
 }
 def OMPC_Private : Clause<"private"> {
   let clangClass = "OMPPrivateClause";
   let flangClass = "OmpObjectList";
 }
 def OMPC_Sizes: Clause<"sizes"> { let clangClass = "OMPSizesClause"; }
-def OMPC_Full: Clause<"full"> { let clangClass = "OMPFullClause"; }
-def OMPC_Partial: Clause<"partial"> { let clangClass = "OMPPartialClause"; }
+def OMPC_Full: Clause<"full"> { 
+  let clangClass = "OMPFullClause"; 
+}
+def OMPC_Partial: Clause<"partial"> { 
+  let clangClass = "OMPPartialClause";
+  let flangClass = "ScalarIntConstantExpr"; 
+  let isValueOptional = true;
+ }
 def OMPC_FirstPrivate : Clause<"firstprivate"> {
   let clangClass = "OMPFirstprivateClause";
   let flangClass = "OmpObjectList";
 }
 def OMPC_LastPrivate : Clause<"lastprivate"> {
   let clangClass = "OMPLastprivateClause";
   let flangClass = "OmpObjectList";
 }