ABataev added a comment. In D71179#1776487 <https://reviews.llvm.org/D71179#1776487>, @hfinkel wrote:
> In D71179#1776467 <https://reviews.llvm.org/D71179#1776467>, @ABataev wrote: > > > In D71179#1776457 <https://reviews.llvm.org/D71179#1776457>, @jdoerfert > > wrote: > > > > > > You're doing absolutely the same thing as the original declare variant > > > > implementation. > > > > > > I don't think so but if you do why do you oppose this approach? > > > > > > > And I don't think it would be correct to add them as multiversiin > > > > variants to the original function. > > > > > > Why wouldn't it be correct to pick the version through the overload > > > resolution instead of the code generation? > > > How this could work is already described in the TODO (CodeGenModule.cpp): > > > > > > // TODO: We should introduce function aliases for `omp declare variant` > > > // directives such that we can treat them through the same > > > overload > > > // resolution scheme (via multi versioning) as `omp begin declare > > > // variant` functions. For an `omp declare variant(VARIANT) ...` > > > // that is attached to a BASE function we would create a global > > > alias > > > // VARIANT = BASE which will participate in the multi version > > > overload > > > // resolution. If picked, here is no need to emit them explicitly. > > > > > > > > > > > > > > > --- > > > > > > I still haven't understood why we cannot/should not reuse the existing > > > multi-version support and instead duplicate the logic in some custom > > > scheme. > > > We have this patch that shows how we can reuse the logic in Clang. It > > > works on a per-call basis, so it will work for all context selector > > > (incl. construct). > > > If you think there is something conceptually not working, I'd like to > > > hear about it. However, just saying "it wouldn't be correct" is not > > > sufficient. You need to provide details about the situation, what you > > > think would not work, and why. > > > > > > I explayned already: declare variant cannot be represented as mutiversion > > functiin, for example. > > > @ABataev, can you please elaborate? It's not obvious to me that we cannot > handle the existing declare variant with the same scheme (as @jdoerfert > highlighted above). In general, I believe it's preferable to have one generic > scheme and use it to handle all cases as opposed to continuing to use a > more-limited scheme in addition to the generic scheme. Eaine already. Current version of declare variant cannot be represented as multiversiin functions, because it is not. We have a function that is the alias to other functions with different names. They just are not multiversion functions by definition. 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
