[PATCH] D83636: omp: Make OMP tablegen more like all other tablegens.

[Valentin Clement via Phabricator via llvm-commits]

clementval added a comment.

In D83636#2152032 <https://reviews.llvm.org/D83636#2152032>, @jdoerfert wrote:

> In D83636#2148958 <https://reviews.llvm.org/D83636#2148958>, @clementval wrote:
>
> > In D83636#2147351 <https://reviews.llvm.org/D83636#2147351>, @thakis wrote:
> >
> > > It puts the C++ code in C++ files :)
> >
> >
> > The C++ code is currently in a C++ files (`OMP.cpp` that replaces `OMPConstants.cpp`). I don't get this comment.
>
>
> We currently generate C++ code via tablegen, now we don't (generate as much C++ code but have the boilerplate manually written in OMP.cpp).
>
> >> I agree there are merits to both approaches, but this is the one used ~everywhere else in LLVM and clang, so if it's a wash it seems good to stay consistent with the rest of the codebase.
> > 
> > MLIR is using similar technics for the dialects code. As I said, I don't have a strong opinion on this but I would like @jdoerfert to have look at this before going forward if you don't mind. It feels just weird to me to re-generate macros to be used to generate code that we can generate directly.
>
> OK, I do not have much tablegen experience nor a strong opinion on this either way. To me, having macros for switches is not the problem since we generate the content via tablegen and not the macro cascade we used before. That makes the difference wrt. type safety and complex relationships. That said, it seems there are certain things we still generate as C++ and then include, e.g., the stuff guarded by `GET_IS_ALLOWED_CLAUSE_FOR_DIRECTIVE_IMPL`. So at the end we replace mostly switch code with generated macros, and use a manually written C++ boilerplate instead of generating it. Both seem sensible given the code size/complexity reduction. And then there is the fact that, if all things are equal, matching LLVM is for sure the preferred way.


There are some limitation to this approach. For example how to you generate the new directive-clauses map used in Flang and that will probably make its way to Clang at some point. I'm just wondering if we are not removing some code for later introducing more complexity. But as I said before I'm happy with whatever is decided.


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

https://reviews.llvm.org/D83636