[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 9 13:59:01 PST 2019


ABataev added a comment.

In D71179#1776034 <https://reviews.llvm.org/D71179#1776034>, @jdoerfert wrote:

> > Not always. If we see that the context selector does not match, we can skip everything between begin/end. It means exactly what I said - multiversioning is needed only for `construct` because all other traits can be easily resolved at the compile time. Generally speaking, there are 2 kinds of traits - global traits (like `vendor`, `kind`, `isa`, etc.), which can be resolved completely statically and do not need multiversioning, and local traits, like `construct`, which depend on the OpenMP directives and require something similar to the multiversioning.
>
> The case where the code is skipped is easy, sure. However, if we "could easily resolve" the other case, we could have implemented an #ifdef solution for `math.h/cmath`. This was not the case and still is not. We basically populate the namespace with multiple versions of the same function (with the same name) and then select the appropriate one for each call site.
>
> Instead of trying to argue why this is not needed for some cases, could you argue why we should have multiple schemes to resolve all types of variants? It seems you inherently assume the codegen patching scheme implemented right now is useful even if we need something else to complement it. I don't think so, thus there is little reason for me to distinguish between the types of variants that need multi-version support ant the types that can be implemented with multi-versions but don't need it.


Because each particular problem requires its own solution and it is always a bad idea to use the microscope to hammer the nails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179





More information about the cfe-commits mailing list