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

clementval marked an inline comment as done. clementval added a comment. In D81736#2117858 , @thakis wrote: > This seems to cause build errors sometimes when buildling libclang. Maybe > there are more missing dependencies? Example failure: > https://logs

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

thakis added a comment. This seems to cause build errors sometimes when buildling libclang. Maybe there are more missing dependencies? Example failure: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8876400305251533648/+/steps/gclient_runhooks/0/stdout Repositor

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

clementval marked 3 inline comments as done. clementval added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:197 } return DKind < OMPD_unknown ? static_cast(DKind) : OMPD_unknown; vdmitrie wrote: > Should this be

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

vdmitrie added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:197 } return DKind < OMPD_unknown ? static_cast(DKind) : OMPD_unknown; Should this be a comparison against llvm::omp::Directive_enumSize rather than

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

clementval marked 3 inline comments as done. clementval added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen)

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

thakis added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen) jdoerfert wrote: > clementval wrote: > > thakis

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

clementval marked 2 inline comments as done. clementval added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen)

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

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen) thakis wrote: > All other tblgen outputs are

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

thakis added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen) All other tblgen outputs are called .inc, not .h

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

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. If you feel reasonably confident that each new DEPENDS is needed, then this LGTM. Otherwise, give it a day to see if anyone with stronger cmake skills than me has a comment. Repository: r

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

clementval added a comment. In D81736#2107202 , @jdenny wrote: > My cmake skills are lacking. Why are there are so many new DEPENDS > relationships where there were none before? Is it because omp_gen is > generating a header file that's included (indir

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

jdenny added a comment. My cmake skills are lacking. Why are there are so many new DEPENDS relationships where there were none before? Is it because omp_gen is generating a header file that's included (indirectly) in all those places, where apparently that sort of dependency was unusual? Hav

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

clementval requested review of this revision. clementval added a comment. @jdoerfert @jdenny I had to add a bunch of dependencies so that the file is generated correctly when needed. Do you have any feedback on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

clementval reopened this revision. clementval added a comment. This revision is now accepted and ready to land. Missing dependencies Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/ https://reviews.llvm.org/D81736 ___

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

jdoerfert accepted this revision. jdoerfert added a comment. Happy. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/ https://reviews.llvm.org/D81736 ___ cfe-commits mailing list cfe-commit

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

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. In D81736#2103822 , @clementval wrote: > @jdoerfert @jdenny Should we wait until Monday to go ahead with this patch? I > have several other patches tha

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

clementval added a comment. @jdoerfert @jdenny Should we wait until Monday to go ahead with this patch? I have several other patches that will follow this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/ https://reviews.llvm.org/D8173

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

jdenny added a comment. Thanks for addressing my comments. As @jdoerfert says, let's give others time to review. Comment at: llvm/test/TableGen/directive1.td:1 +// RUN: llvm-tblgen -gen-directive-decls -I %p/../../include %s | FileCheck %s + I realize that or

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

clementval marked 10 inline comments as done. clementval added a comment. @jdenny I will have a look at your suggestion to dump the name instead of the id. If it looks straight forward I will do it. Other comments were addressed. Thanks. Comment at: clang/test/AST/ast-dump-op

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

jdenny added a comment. This patch looks like a good idea to me. Thanks. In 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 enu

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

clementval added a comment. In D81736#2096336 , @jdoerfert wrote: > In D81736#2095947 , @clementval > wrote: > > > In D81736#2093926 , @jdoerfert > > wrote: > > > > > Assum

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

jdoerfert added a comment. In D81736#2095947 , @clementval wrote: > In D81736#2093926 , @jdoerfert wrote: > > > Assuming this passes all the tests, it looks good to me. Let's wait a day > > or two for people to tak

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

clementval added a comment. In D81736#2093926 , @jdoerfert wrote: > Assuming this passes all the tests, it looks good to me. Let's wait a day or > two for people to take a look though :) @jdoerfert Sure let's wait a bit. I see a failure because of some

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

jdoerfert added a comment. Assuming this passes all the tests, it looks good to me. Let's wait a day or two for people to take a look though :) Comment at: llvm/test/TableGen/directive.td:34 +// CHECK-NEXT: } +// CHECK-NEXT: } clementval wrote: > jdoerfert wro

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

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: >

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

jdoerfert added reviewers: fghanim, ABataev, jdenny, hfinkel, jhuber6, kiranchandramohan, kiranktp. jdoerfert added a comment. This is really cool :) In D81736#2090419 , @clementval wrote: > @jdoerfert Some tests in clang relies on the position of the sp

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

clementval updated this revision to Diff 270481. clementval added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Keep same ordering than OMPKinds.def + add dependency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8