[PATCH] D79410: [MLIR] [OpenMP] Add basic OpenMP parallel operation

Tue May 5 15:40:41 PDT 2020

kiranchandramohan added inline comments.

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:57

+def ClauseDefaultPrivate : StrEnumAttrCase<"defprivate">;
+def ClauseDefaultFirstPrivate : StrEnumAttrCase<"deffirstprivate">;
----------------
rriddle wrote:
> nit: Can you add more documentation for what these correspond to?
These are all values for the default clause : private, firstprivate, shared, none. Had to add the 'def' prefix since there was some error in the code generated from the spec because private is a keyword in c++.  
Is there another way to solve this and keep the clause values as private rather than defprivate?

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:60
+def ClauseDefaultShared : StrEnumAttrCase<"defshared">;
+def ClauseDefaultNone : StrEnumAttrCase<"defnone">;
+
----------------
DavidTruby wrote:
> jdoerfert wrote:
> > If the preprocessor runs on this file you can use the OMPKinds.def macros to avoid listing stuff explicitly here. That also keeps everything in sync.
> I'll be honest, I have absolutely no idea how tablegen works. Will the preprocessor be run on files generated by this? I assume it will but I don't really know....
I am guessing it will run on this file because of the presence of include and #define at the top of this file (lines 14-17).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79410