ABataev added a comment.

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

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


I have different opinion. You can reuse existing codegen for declare variant 
functions with global context selectors only. You just need to iterate through 
all the variants and choose the best one.
That's why you don't need the dispatching in your scheme. You're doing 
absolutely the same thing as the original declare variant implementation.

We cannot use multiversioning for the original declare variant construct since 
there is no multiversioning at all. We have a single function with many 
different aliasing functions, having different names. They are completely 
different functions. And I don't think it would be correct to add them as 
multiversiin variants to the original function.


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

Reply via email to