jdoerfert added a comment. In D71241#1779097 <https://reviews.llvm.org/D71241#1779097>, @ABataev wrote:
> In D71241#1778963 <https://reviews.llvm.org/D71241#1778963>, @jdoerfert wrote: > > > In D71241#1778736 <https://reviews.llvm.org/D71241#1778736>, @ABataev wrote: > > > > > In D71241#1778717 <https://reviews.llvm.org/D71241#1778717>, @jdoerfert > > > wrote: > > > > > > > >> There is no evidence that this is more complicated. By all measures, > > > > >> this is less complicated (see also below). It is also actually doing > > > > >> the right thing when it comes to code emission. Take > > > > >> https://godbolt.org/z/sJiP3B for example. The calls are wrong and > > > > >> the definition of base is missing. > > > > > > > > > > How did you measure it? I have a completely different opinion. Also, > > > > > tried to reproduce the problem locally, could not reproduce. It seems > > > > > to me, the output of the test misses several important things. You > > > > > can check it yourself. `tgt_target_teams()` call uses > > > > > `@.offload_maptypes` global var but it is not defined. > > > > > > > > Here is the link with the globals not hidden: > > > > https://godbolt.org/z/5etB5S > > > > The base function is called both times but should not be called at > > > > all. What is your local output and why does it differ? > > > > > > > > > On the host `base` is an alias for the `hst` function. On the device > > > `base` has the body of `dev` function because NVPTX does nit support > > > function aliases (10+ suppprts it, but LLVM does not support it yet). Try > > > to change the bodies of dev and hst and you will see. > > > > > > I tried to keep original function names to improve debugging and make > > > users less wonder why instead of `base` something else is called. > > > > > > How does that work with linking? Another translation unit can call both the > > base and hst/dev function, right? I mean they both need to be present. > > > If hst or dev are needed, they are emitted too, independently. On the host, > the variamt function is emitted and base function is set to be an alias of > the variant function. On the device we just inherit the body of the variant > function, but this variant function also can be emitted, if used > independently. This is confusing: 1. Especially for debugging we should do the same on host and device. 2. The device function now exists twice, that is bad. 3. How is this supposed to work with type corrections? I mean the variant needs to be compatible but not necessarily of the same type, right? https://godbolt.org/z/QAXCuv we just produce a cryptic error (locally it crashes for me afterwards). 4. On the host the expression `&base == &hst` will evaluate to true with the alias. In D71241#1779779 <https://reviews.llvm.org/D71241#1779779>, @ABataev wrote: > Here is the example that does not work with the proposed solution but works > with the existing one: > > static void cpu() { asm("nop"); } > > #pragma omp declare variant(cpu) match(device = {kind(cpu)}) > static __attribute__((used)) void wrong_asm() { > asm ("xxx"); > } > > > The existing solution has some problems with the delayed error messages too, > but they are very easy to fix. I don't understand this example. What is the expected outcome here (I get the error below from ToT clang). Why is that not achievable by a SemaOverload solution? `asm.c:4:35: error: function with '#pragma omp declare variant' must have a prototype` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71241/new/ https://reviews.llvm.org/D71241 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
