hfinkel added a comment. In D71179#1776491 <https://reviews.llvm.org/D71179#1776491>, @ABataev wrote:
> 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. I understand that they have different names. I don't see why we that means that they can't be added to the overload set as multi-version candidates if we add logic which does exactly that. 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
