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

Mon Dec 9 14:26:38 PST 2019

jdoerfert added a comment.

In D71179#1776046 <https://reviews.llvm.org/D71179#1776046>, @ABataev wrote:

> 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.

While I see where you are coming from, I disagree. We have a generic framework available that we already need to use in some cases, there is no harm in using it for all cases. It would be different if we wouldn't need the generic framework at all, but that is not the case. All I ask is to literally share existing code, no additional complexity needed. Your suggestion will complicate the setup, duplicate logic, and make it overall harder to maintain and compose in the future. If you still disagree, please provide some arguments (and details) why we would benefit from your setup.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179