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

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179