[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added a comment. In D77954#2021299 , @yaxunl wrote: > I will put a workaround: In device compilation, in implicit `__device__ > __host__` callers, I will keep the old behavior, that is, implicit > `__device__ __host__` candidate has equal preference

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added a comment. FYI, I've just reverted it in bf6a26b066382e0f41bf023c781d84061c542307 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl added a comment. In D77954#2021026 , @tra wrote: > It appears that re-landed b46b1a916d44216f0c70de55ae2123eb9de69027 > has > created another compilation regression. I don't

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added a comment. It appears that re-landed b46b1a916d44216f0c70de55ae2123eb9de69027 has created another compilation regression. I don't have a simple reproducer yet, so here's the error message for now:

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl added a comment. In D77954#2005313 , @gribozavr2 wrote: > Sorry -- this change broke overload resolution for `operator new`, as it is > declared in system headers. I'm reverting the patch. > > $ cat /tmp/in.cu.cc > #define __device__

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

gribozavr2 added a comment. Sorry -- this change broke overload resolution for `operator new`, as it is declared in system headers. I'm reverting the patch. $ cat /tmp/in.cu.cc #define __device__ __attribute__((device)) void* operator new(__SIZE_TYPE__ size); __device__ void *operator

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl added a comment. In D77954#2002580 , @tra wrote: > Go ahead. I'll revert it if it breaks anything on our side. Thanks. Done by b46b1a916d44216f0c70de55ae2123eb9de69027

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added a comment. Go ahead. I'll revert it if it breaks anything on our side. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 ___ cfe-commits mailing list

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

This revision was automatically updated to reflect the committed changes. Closed by commit rGc77a4078e010: [CUDA][HIP] Fix host/device based overload resolution (authored by yaxunl). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl added a comment. @tra Is it OK I commit it now? Or better wait next Monday morning? Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 ___ cfe-commits mailing list

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall accepted this revision. rjmccall added a comment. Thanks, Yaxun. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + yaxunl wrote: > rjmccall wrote: > > tra wrote: > > > rjmccall wrote: > > > > erichkeane wrote: > > > > > yaxunl

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl updated this revision to Diff 259870. yaxunl marked an inline comment as done. yaxunl added a comment. change the precedence of multiversion to be over host/device-ness. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 Files:

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl marked 6 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + rjmccall wrote: > tra wrote: > > rjmccall wrote: > > > erichkeane

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl updated this revision to Diff 259796. yaxunl marked an inline comment as done. yaxunl added a comment. Revised by John's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 Files: clang/lib/Sema/SemaOverload.cpp

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + tra wrote: > rjmccall wrote: > > erichkeane wrote: > > > yaxunl wrote: > > > > echristo wrote: > > > > >

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra accepted this revision. tra added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + rjmccall wrote: > erichkeane wrote: >

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + erichkeane wrote: > yaxunl wrote: > > echristo wrote: > > > rjmccall wrote: > > > > yaxunl wrote: > > > > >

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + yaxunl wrote: > echristo wrote: > > rjmccall wrote: > > > yaxunl wrote: > > > > rjmccall wrote: > > > > >

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + echristo wrote: > rjmccall wrote: > > yaxunl wrote: > > > rjmccall

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

echristo added a subscriber: erichkeane. echristo added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9749 + if (isBetterMultiversionCandidate(Cand1, Cand2)) +return true; + rjmccall wrote: > yaxunl wrote: > > rjmccall wrote: > > > If we move

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall added a comment. Okay, one minor fix. Comment at: clang/lib/Sema/SemaOverload.cpp:9389 + if (Cand2.Function->isInvalidDecl()) +return Comparison::Better; This is neglecting the case where they're both invalid. CHANGES SINCE LAST ACTION

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl updated this revision to Diff 259458. yaxunl added a comment. Revised to let host/device take precedence over multiversion, as John suggested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 Files: clang/lib/Sema/SemaOverload.cpp

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/ https://reviews.llvm.org/D77954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

tra added a comment. LGTM in principle. That said, my gut feeling is that this patch has a good chance of breaking something in sufficiently convoluted CUDA code like Eigen. When you land this patch, I'd appreciate if you could do it on a workday morning (Pacific time) so I'm around to test it

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall added a reviewer: echristo. rjmccall added a subscriber: echristo. rjmccall added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9481 + // emitted, Cand1 is not better than Cand2. This rule should have precedence + // over other rules. + //

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl updated this revision to Diff 256973. yaxunl marked 3 inline comments as done. yaxunl added a comment. fix preference for multiversion. add comments. add more tests for wrong-sided function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77954/new/

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl marked 6 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9481 + // emitted, Cand1 is not better than Cand2. This rule should have precedence + // over other rules. + // rjmccall wrote: > Please add

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:9481 + // emitted, Cand1 is not better than Cand2. This rule should have precedence + // over other rules. + // Please add `[CUDA]` or something similar to the top of this comment so

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

yaxunl updated this revision to Diff 256903. yaxunl retitled this revision from "[CUDA][HIP] Fix overload resolution issue for device host functions" to "[CUDA][HIP] Fix host/device based overload resolution". yaxunl added a comment. Revised by John's comments. CHANGES SINCE LAST ACTION