[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 3 inline comments as done.
clementval added inline comments.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496
+def OMP_EndDeclareVariant : Directive<"end declare variant"> {}
+def OMP_Unknown : Directive<"unknown"> {}
----------------
jdoerfert wrote:
> I guess we go over this in a follow up and use the `requiredClauses` and similar features, that seems reasonable to me. 
Yes, that was the plan. I tried to stick to the way it is done today. Then we can use the extra abstraction when more things are in place. 


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:50
-#include "llvm/Frontend/OpenMP/OMPKinds.def"
-
 /// IDs for all omp runtime library (RTL) functions.
----------------
jdoerfert wrote:
> While the MACRO MAGIC worked surprisingly well, I'm glad it is going away.
I hope we can replace as much as possible the MACRO MAGIC. We might have to keep some of it were there is no clear abstraction. Let's see.  


================
Comment at: llvm/test/TableGen/directive.td:34
+// CHECK-NEXT: }
+// CHECK-NEXT: }
----------------
jdoerfert wrote:
> How does the `allowedClauses` affect the result?
> 
It does not affect the result in this case. It will be used in a next patch were with can replace the `isAllowedClauseForDirective` code generation with TableGen. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736