[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 13 04:27:04 PST 2019


ABataev added a comment.

In D71241#1782963 <https://reviews.llvm.org/D71241#1782963>, @jdoerfert wrote:

> In D71241#1782668 <https://reviews.llvm.org/D71241#1782668>, @ABataev wrote:
>
> > In D71241#1782650 <https://reviews.llvm.org/D71241#1782650>, @jdoerfert wrote:
> >
> > > While we talk a lot about what you think is bad about this solution it seems we ignore the problems in the current one. Let me summarize a few:
> > >
> > > - Take https://godbolt.org/z/XCjQUA where the wrong function is called in the target region (because the "hack" to inject code in the wrong definition is not applicable).
> >
> >
> > No time for it, just short answers. No definition for the variant - no definition for the base.
>
>
> This is perfectly valid code and with the current scheme impossible to support.
>
> >> - Take https://godbolt.org/z/Yi9Lht where the wrong function is called on the host (no there is *no* alias hidden)
> > 
> > GlobalAlias can be emitted only for definitions. No definition for variant - no aliasing.
>
> Exactly, as above, this is a problem.
>
> >> - Take https://godbolt.org/z/2evvtN which shows that the alias solution is incompatible with linking.
> > 
> > Undefined behavior according to the standard.
>
> I don't think so. If you do, please reference the rules this would violate.


Page 59, 25-27.

> 
> 
>>> - Take the `construct` context selector and the `begin/end declare variant` construct which both cannot be implemented with aliases.
> 
> This can also not be implemented in the alias scheme.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241





More information about the cfe-commits mailing list