[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration
Johannes Doerfert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 13 09:38:30 PDT 2020
jdoerfert added reviewers: fghanim, ABataev, jdenny, hfinkel, jhuber6, kiranchandramohan, kiranktp.
jdoerfert added a comment.
This is really cool :)
In D81736#2090419 <https://reviews.llvm.org/D81736#2090419>, @clementval wrote:
> @jdoerfert Some tests in clang relies on the position of the specific enum in the declaration. TableGen is sorting the records so the enum is generated in alphabetical order therefore the enum position is changed. Is it fine to update the test with smith like `{{.*}}` instead of the specific id?
Yes, that's fine.
I left some comments and added some more reviewers.
================
Comment at: llvm/include/llvm/Frontend/Directive/DirectiveBase.td:69
+ list<Clause> requiredClauses = ?;
+}
----------------
I really like we have this abstraction now and also a nice way to encode some constraints, e.g., required clauses, right away :)
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496
+def OMP_EndDeclareVariant : Directive<"end declare variant"> {}
+def OMP_Unknown : Directive<"unknown"> {}
----------------
I guess we go over this in a follow up and use the `requiredClauses` and similar features, that seems reasonable to me.
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:50
-#include "llvm/Frontend/OpenMP/OMPKinds.def"
-
/// IDs for all omp runtime library (RTL) functions.
----------------
While the MACRO MAGIC worked surprisingly well, I'm glad it is going away.
================
Comment at: llvm/test/TableGen/directive.td:34
+// CHECK-NEXT: }
+// CHECK-NEXT: }
----------------
How does the `allowedClauses` affect the result?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81736/new/
https://reviews.llvm.org/D81736
More information about the cfe-commits
mailing list