hfinkel added a comment. In D71241#1782779 <https://reviews.llvm.org/D71241#1782779>, @ABataev wrote:
> In D71241#1782742 <https://reviews.llvm.org/D71241#1782742>, @hfinkel wrote: > > > In D71241#1782703 <https://reviews.llvm.org/D71241#1782703>, @ABataev wrote: > > > > > > > > > > > ... > > > > >>>>>> > > >>>>>> > > >>>>>>> Given we have two implementations, each at different points in the > > >>>>>>> pipeline, it might be constructive to each write down why you each > > >>>>>>> choose said point. I suspect the rationale is hidden by the > > >>>>>>> implementation details. > > >>>>> > > >>>>> Actually, early resolution will break tbe tools, not help them. It > > >>>>> will definitely break clangd, for example. The user will try to > > >>>>> navigate to originally called function `base` and instead he will be > > >>>>> redirected to the function `hst`. > > >>>> > > >>>> Can you please be more specific? Please explain why the user would > > >>>> consider this incorrect behavior. If the point of the tool is to allow > > >>>> the user to navigate to the function actually being called, then > > >>>> navigating to `base` seems incorrect if that's not the function being > > >>>> called. This is just like any other kind of overload resolution - the > > >>>> user likely wants to navigate to the function being called. > > >>>> > > >>>> Now the user might want an OpenMP-aware tool that understands > > >>>> differences between host and accelerator behavior, how that affects > > >>>> which functions are called, etc. The user might want this for > > >>>> host/device overloads in CUDA too, but this is really an orthogonal > > >>>> concern. > > >>> > > >>> You wrote the code. You called a function in the expression. Now you > > >>> want to navivate to this function. Clicked on it and instead of the > > >>> called `base` you are redirected to `hst` because AST has the link to > > >>> `hst` functiin inthe expression instead of the `base`. > > >> > > >> Sure, but it has that link because that `hst` function is actually the > > >> function being called (assuming that the clangd-using-tool is configured > > >> to interpret the code as if compiling for the host). When I click on a > > >> function call in a source file, I want to navigate to the function > > >> actually being called. I certainly understand that the function being > > >> called now depends on compilation context, and the tool in our example > > >> is only providing the resolution in one context, but at least it > > >> provides one valid answer. An OpenMP-aware tool could navigate to the > > >> base function (we do need to preserve information to make this > > >> possible). This is just like dealing with some host/device functions in > > >> CUDA (where there are separate overloads) - if you click on the function > > >> in such a tool you'll probably navigate to the host variant of the > > >> function (even if, in some other context, the device overload might be > > >> called). > > >> > > >> Again, I see this as exactly analogous to overload resolution, or as > > >> another example, when calling a function template with specializations. > > >> When using such a tool, my experience is that users want to click on the > > >> function and navigate to the function actually being called. If, for > > >> example, I have a function template with specializations, if the > > >> specialized one is being called, I should navigate to the specialization > > >> being called (not the base function template). > > > > > > You wrote wrong context matcher. You has a bug in the base function, > > > which should be called by default everu sw here but the host, and want to > > > fix it. Etc. > > > > I understand, but this is a generic problem. Same with host/device > > overloads in CUDA. Your tool only gets one compilation context, and thus > > only one callee. In addition, FWIW, having a base version called everywhere > > except on the host seems like an uncommon use case. Normally, the base > > version *is* the version called on the host. The named variants are likely > > the ones specialized for various accelerators. > > > > Regardless, this is exactly why we should do this in Sema. We can represent > > links to both Decls in the AST (as I indicated in an earlier comment), and > > then it will be *possible* for an OpenMP-aware tool to decide on which it > > wants. Right now, it's not easily possible to write a tool that can use an > > appropriate set of contexts to examine the AST where the actual callees are > > represented. > > > No need to worry for the right decl. See D7097 > <https://reviews.llvm.org/D7097>. If you see a refernce for function, before > doing something with it, just call member function > `getOpenMPDeclareVariantFunction()` and you get the function that must be > actually called here. The tool must do the same. That's hiw the tools work. > They must be aware of special costructs/attributes. But then we'd to add code to do that in Clang's static analyzer and all other code trying to match caller/callee relationships. The function provided in the AST being not the function that will actually be called. Instead, we should make these tools correct by default and make tools wanting to to OpenMP-specific things be the tools that need to call the OpenMP-specific functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71241/new/ https://reviews.llvm.org/D71241 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
