[PATCH] D86071: [MLIR][OpenMP] Add omp.wsloop operation

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:126
+def OMP_SCHEDULE_Runtime : ClauseVal<"Runtime", 6, 1> {}
+def OMP_SCHEDULE_Default : ClauseVal<"Default", 7, 0> { let isDefault = 1; }
+
----------------
To detect some invalid keyword, there should be some unknown constant as well. Otherwise the front-end must itself store a complete list of valid schedule arguments.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:120
+
+// static and auto are C++ keywords so need a _ to disambiguate
+def OMP_SCHEDULE_static_ : ClauseVal<"static_",2,1> {}
----------------
kiranchandramohan wrote:
> clementval wrote:
> > kiranchandramohan wrote:
> > > For the default clause in parallel we have used a prefix "def" to fix this issue. I think we need to standardize this. Would converting the first letter to caps be a reasonable workaround since reserved keywords do not have a letter with caps as first letter?
> > That's probably a good idea to have a standard way to do that. +1 for the first letter capitalized if it works. 
> I have filed an issue https://bugs.llvm.org/show_bug.cgi?id=47225 regarding StrEnumAttr not accepting reserved C++ keywords.
IIUC, the are meant to be directly passed by the frontend to get the schedule mode:
```
getScheduleKind(clausearg.str())
```
I.e. this would require the user to write:
```
#pragma omp for schedule(Static)
```
since 
```
#pragma omp for schedule(static)
```
will give you `OMP_SCHEDULE_Default`. See D91643.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86071