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

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

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



================
Comment at: flang/lib/Parser/openmp-parsers.cpp:256
+    "PARTIAL" >> construct<OmpClause>(construct<OmpClause::Partial>(
+                     maybe(parenthesized(scalarIntExpr)))) ||                 
     "PRIORITY" >> construct<OmpClause>(construct<OmpClause::Priority>(
----------------
I think `scalarIntConstantExpr` is the correct one to use here.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:76
+  let clangClass = "OMPPartialClause";
+  let flangClass = "ScalarIntExpr"; 
+  let isValueOptional = true;
----------------
kiranchandramohan wrote:
> 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.
> ```
This comment is not addressed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138229/new/

https://reviews.llvm.org/D138229