[PATCH] D82405: [openmp] Move Directive and Clause helper function to tablegen

Fri Jun 26 12:35:59 PDT 2020

clementval added a comment.

In D82405#2117219 <https://reviews.llvm.org/D82405#2117219>, @jdenny wrote:

> In D82405#2112522 <https://reviews.llvm.org/D82405#2112522>, @clementval wrote:
>
> > In D82405#2112482 <https://reviews.llvm.org/D82405#2112482>, @jdoerfert wrote:
> >
> > > In D82405#2112451 <https://reviews.llvm.org/D82405#2112451>, @clementval wrote:
> > >
> > > > In D82405#2112272 <https://reviews.llvm.org/D82405#2112272>, @jdoerfert wrote:
> > > >
> > > > > So the idea is we generate a single .inc file with multiple `ifdef` regions to choose from? Is that what is usually done? I would have thought multiple files are nicer but no strong feeling.
> > > >
> > > >
> > > > I have no strong opinion on this as well. MLIR is generating two files for their dialects (1 ``.h.inc` and `.cpp.inc) but use the sections as well. We will probably need the sections as well even if we do two or more files.
> > >
> > >
> > > Why do we need sections if we generate multiple files?
> >
> >
> > So far if we have a .h.inc and a .cpp.inc there is no need for sections for the common parts related to enumerations but I can imagine that some part in clang or Flang which currently use OMPKinds.def need replacement code that are quite specific and might reside in a section. Does it make sense?
>
>
> That would be for tweaking parts of the content not for selecting entirely different content, right?

That's used for selecting different part. It is done like this both in pure TableGen and miler-tablegen uses.

> It seems unintuitive to me to force two completely different files into one file in this manner, but I have no strong opinion either.  If that's the norm, then fine by me.

As I said, I have no strong opinion but we could at least keep a .h.inc file clean if we choose a two files approache.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82405