hfinkel added a comment. In D71179#1775157 <https://reviews.llvm.org/D71179#1775157>, @ABataev wrote:
> In D71179#1775066 <https://reviews.llvm.org/D71179#1775066>, @hfinkel wrote: > > > In D71179#1774678 <https://reviews.llvm.org/D71179#1774678>, @ABataev wrote: > > > > > In D71179#1774487 <https://reviews.llvm.org/D71179#1774487>, @jdoerfert > > > wrote: > > > > > > > In D71179#1774471 <https://reviews.llvm.org/D71179#1774471>, @ABataev > > > > wrote: > > > > > > > > > They do this because they have several function definitions with the > > > > > same name. In our case, we have several different functions with > > > > > different names and for us no need to worry about overloading > > > > > resolution, the compiler will do everything for us. > > > > > > > > > > > > I think we talk past each other again. This is the implementation of > > > > `omp begin/end declare variant` as described in TR8. Bt definition, the > > > > new variant mechanism will result in several different function > > > > definitions with the same name. See the two tests for examples. > > > > > > > > > I just don't get it. If begin/end is just a something like > > > #ifdef...endif, why you just can't skip everything between begin/end if > > > the context does not match? > > > > > > The patch does this (see in ParseOpenMP.cpp where I asked about the > > potential inf-loop). But when the definitions are not skipped, then we have > > to worry about having multiple decls/defs of the same name and the overload > > priorities. > > > I would recommend to drop all this extra stuff from the patch and focus on > the initial patch. We'll need something similar to multiversion in case of > the construct context selectors, but at first we need to solve all the > problems with the simple versions of the construct rather that try to solve > all the problems in the world in one patch. It is almost impossible to review. I agree. We should split this into several patches (e.g., basic handling, skipping parsing for incompatible selectors, overload things). I think that @jdoerfert posted this so that people can see the high-level direction and provide feedback (including feedback on how to stage the functionality for review). @jdoerfert , also, do we have tests that can go into the test suite / libomptarget regression tests demonstrating the collection of problems people have currently opened bugs on regarding math.h? I recall we still had problems with host code needing the long-double overloads, with constants from the system headers, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71179/new/ https://reviews.llvm.org/D71179 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
