[PATCH] D67730: [CUDA][HIP] Fix typo in `BestViableFunction`

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added a reviewer: tra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Should consider viable ones only when checking SameSide candidates.
- Replace erasing with clearing viable flag to reduce data moving/copying.
- Add one and revise another one as the diagnostic message are more relevant compared to previous one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67730

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/implicit-member-target-collision-cxx11.cu


Index: clang/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
===================================================================
--- clang/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
+++ clang/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
@@ -74,11 +74,13 @@
 struct C4_with_collision : A4_with_host_copy_ctor, B4_with_device_copy_ctor {
 };
 
-// expected-note at -3 {{copy constructor of 'C4_with_collision' is implicitly deleted because base class 'B4_with_device_copy_ctor' has no copy constructor}}
+// expected-note at -3 {{candidate constructor (the implicit copy constructor) not viable: call to invalid function from __host__ function}}
+// expected-note at -4 {{implicit copy constructor inferred target collision: call to both __host__ and __device__ members}}
+// expected-note at -5 {{candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided}}
 
 void hostfoo4() {
   C4_with_collision c;
-  C4_with_collision c2 = c; // expected-error {{call to implicitly-deleted copy constructor of 'C4_with_collision'}}
+  C4_with_collision c2 = c; // expected-error {{no matching constructor for initialization of 'C4_with_collision'}}
 }
 
 //------------------------------------------------------------------------------
Index: clang/test/SemaCUDA/function-overload.cu
===================================================================
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -1,8 +1,8 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
 
 #include "Inputs/cuda.h"
 
@@ -402,3 +402,20 @@
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ variant.
 }
+
+// Two irrelevant classes with `operator-` defined. One of them is device only.
+struct C1 { int m; };
+struct C2 { int *m; };
+__device__
+int operator-(const C1 &x, const C1 &y) { return x.m - y.m; }
+int operator-(const C2 &x, const C2 &y) { return x.m - y.m; }
+
+template <typename T>
+constexpr int constexpr_overload(const T &x, const T &y) {
+  return x - y;
+}
+
+// Verify that function overloading doesn't prune candidate wrongly.
+int test_constexpr_overload(C2 x, C2 y) {
+  return constexpr_overload(x, y);
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9422,17 +9422,19 @@
     const FunctionDecl *Caller = dyn_cast<FunctionDecl>(S.CurContext);
     bool ContainsSameSideCandidate =
         llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
-          return Cand->Function &&
+          // Consider viable function only.
+          return Cand->Viable && Cand->Function &&
                  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
                      Sema::CFP_SameSide;
         });
     if (ContainsSameSideCandidate) {
-      auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
-        return Cand->Function &&
-               S.IdentifyCUDAPreference(Caller, Cand->Function) ==
-                   Sema::CFP_WrongSide;
-      };
-      llvm::erase_if(Candidates, IsWrongSideCandidate);
+      // Clear viable flag for WrongSide varible candidates.
+      llvm::for_each(Candidates, [&](OverloadCandidate *Cand) {
+        if (Cand->Viable && Cand->Function &&
+            S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+                Sema::CFP_WrongSide)
+          Cand->Viable = false;
+      });
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67730.220738.patch
Type: text/x-patch
Size: 3954 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190918/f4c69915/attachment.bin>