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

Wed May 6 09:40:33 PDT 2020

jdoerfert added inline comments.

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:60
+def ClauseDefaultShared : StrEnumAttrCase<"defshared">;
+def ClauseDefaultNone : StrEnumAttrCase<"defnone">;
+
----------------
DavidTruby wrote:
> DavidTruby wrote:
> > kiranchandramohan wrote:
> > > 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).
> > I tried this and it seems like tablegen tries to interpret `#define` and then shouts at me. Any ideas how I can get it to ignore my #define and just put it in the generated code?
> Doesn't look like this is possible due to tablegen having its own custom preprocessor unfortunately. One suggestion I've seen is to make that header be generated by tablegen so that we can use it in tablegen files. Given that this will probably come up again in other constructs that might be worth us doing, but I think it should be done as a separate patch. What do you think @jdoerfert ?
1) A separate patch is generally fine.
2) We should investigate reuse opportunities now, before we duplicate tings ;)
3) We could probably preprocess a file explicitly and include it here *or* move OMPKinds.def to a tablegen file as well (part by part). I did avoid that so far but it might make sense in the long run.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79410