fix review remarks

maksimsab · maksimsab · commit 7d4b97350408 · 2021-12-22T13:56:07.000+03:00
diff --git a/llvm/test/tools/sycl-post-link/assert-indirect-with-split-2.ll b/llvm/test/tools/sycl-post-link/assert-indirect-with-split-2.ll
@@ -9,7 +9,7 @@
 ; marked as using asserts.
 
 ; RUN: sycl-post-link -split=auto -symbols -S %s -o %t.table
-; RUN: FileCheck %s -input-file=%t_0.prop
+; RUN: FileCheck %s -input-file=%t_0.prop --implicit-check-not main_TU0_kernel1
 
 target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
 target triple = "spir64-unknown-linux"
@@ -63,6 +63,12 @@ entry:
   ret void
 }
 
+define dso_local spir_kernel void @main_TU0_kernel1() #0 {
+entry:
+  call spir_func void @_Z4foo1v()
+  ret void
+}
+
 ; Function Attrs: nounwind
 define dso_local spir_func void @_Z4foo1v() {
 entry:
@@ -78,13 +84,6 @@ entry:
   ret void
 }
 
-; CHECK-NOT: main_TU0_kernel1
-define dso_local spir_kernel void @main_TU0_kernel1() #0 {
-entry:
-  call spir_func void @_Z4foo1v()
-  ret void
-}
-
 
 ; This function is marked with "referenced-indirectly", but it doesn't call an assert
 ; Function Attrs: nounwind
diff --git a/llvm/test/tools/sycl-post-link/assert-property-2.ll b/llvm/test/tools/sycl-post-link/assert-property-2.ll
@@ -3,7 +3,7 @@
 ; graph.
 
 ; RUN: sycl-post-link -split=auto -symbols -S %s -o %t.table
-; RUN: FileCheck %s -input-file=%t_0.prop
+; RUN: FileCheck %s -input-file=%t_0.prop --implicit-check-not _ZTSZZ4mainENK3$_0clERN2cl4sycl7handlerEE7Kernel6
 
 ; SYCL source:
 ; void assert_func() {
@@ -267,6 +267,14 @@ entry:
   ret void
 }
 
+; Function Attrs: convergent norecurse mustprogress
+define weak_odr dso_local spir_kernel void @"_ZTSZZ4mainENK3$_0clERN2cl4sycl7handlerEE7Kernel6"() local_unnamed_addr #0 {
+entry:
+  call spir_func void @_Z6E_exclv()
+  call spir_func void @_Z6E_exclv()
+  ret void
+}
+
 ; Function Attrs: convergent norecurse nounwind mustprogress
 define dso_local spir_func void @_Z6F_inclv() local_unnamed_addr {
 entry:
@@ -327,15 +335,6 @@ define weak_odr dso_local spir_kernel void @"_ZTSZZ4mainENK3$_0clERN2cl4sycl7han
   ret void
 }
 
-; CHECK-NOT: _ZTSZZ4mainENK3$_0clERN2cl4sycl7handlerEE7Kernel6
-; Function Attrs: convergent norecurse mustprogress
-define weak_odr dso_local spir_kernel void @"_ZTSZZ4mainENK3$_0clERN2cl4sycl7handlerEE7Kernel6"() local_unnamed_addr #0 {
-entry:
-  call spir_func void @_Z6E_exclv()
-  call spir_func void @_Z6E_exclv()
-  ret void
-}
-
 ; Function Attrs: convergent norecurse mustprogress
 define weak dso_local spir_func void @__assert_fail(i8 addrspace(4)* %expr, i8 addrspace(4)* %file, i32 %line, i8 addrspace(4)* %func) local_unnamed_addr {
 entry:
diff --git a/llvm/test/tools/sycl-post-link/assert-property-with-split.ll b/llvm/test/tools/sycl-post-link/assert-property-with-split.ll
@@ -3,7 +3,7 @@
 ; in their call graph.
 
 ; RUN: sycl-post-link -split=auto -symbols -S %s -o %t.table
-; RUN: FileCheck %s -input-file=%t_0.prop
+; RUN: FileCheck %s -input-file=%t_0.prop --implicit-check-not _ZTSZ4mainE11TU0_kernel1
 
 target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
 target triple = "spir64-unknown-linux"
@@ -36,14 +36,6 @@ entry:
   ret void
 }
 
-; CHECK-DAG: _ZTSZ4mainE10TU1_kernel
-define dso_local spir_kernel void @_ZTSZ4mainE10TU1_kernel() #1 {
-entry:
-  call spir_func void @_Z4foo2v()
-  ret void
-}
-
-; CHECK-NOT: _ZTSZ4mainE11TU0_kernel1
 define dso_local spir_kernel void @_ZTSZ4mainE11TU0_kernel1() #0 {
 entry:
   call spir_func void @_Z4foo1v()
@@ -58,6 +50,13 @@ entry:
   ret void
 }
 
+; CHECK-DAG: _ZTSZ4mainE10TU1_kernel
+define dso_local spir_kernel void @_ZTSZ4mainE10TU1_kernel() #1 {
+entry:
+  call spir_func void @_Z4foo2v()
+  ret void
+}
+
 ; Function Attrs: nounwind
 define dso_local spir_func void @_Z4foo2v() {
 entry:
diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp
@@ -357,14 +357,14 @@ void groupEntryPoints(const Module &M, EntryPointGroupMap &EntryPointsGroups,
 
 // This function traverses over reversed call graph by BFS algorithm.
 // It means that an edge links some function @func with functions
-// which contain call of function @func.It starts from
+// which contain call of function @func. It starts from
 // @StartingFunction and lifts up until it reach all reachable functions
 // or it reaches some function containing "referenced-indirectly" attribute.
-// If it reaches "referenced-indirectly" attribute than it returns true and
-// an empty list.
-// Otherwise, it returns false and a list of reached SPIR kernel function's
-// names.
-std::pair<bool, std::vector<StringRef>>
+// If it reaches "referenced-indirectly" attribute than it returns an empty
+// Optional.
+// Otherwise, it returns an Optional containing a list of reached
+// SPIR kernel function's names.
+Optional<std::vector<StringRef>>
 TraverseCGToFindSPIRKernels(const Function *StartingFunction) {
   std::queue<const Function *> FunctionsToVisit;
   std::unordered_set<const Function *> VisitedFunctions;
@@ -375,21 +375,20 @@ TraverseCGToFindSPIRKernels(const Function *StartingFunction) {
     const Function *F = FunctionsToVisit.front();
     FunctionsToVisit.pop();
 
+    auto InsertionResult = VisitedFunctions.insert(F);
     // It is possible that we insert some particular function several
     // times in functionsToVisit queue.
-    if (VisitedFunctions.find(F) != VisitedFunctions.end())
+    if (!InsertionResult.second)
       continue;
 
-    VisitedFunctions.insert(F);
-
     for (const auto *U : F->users()) {
       const Instruction *I = cast<const Instruction>(U);
       const Function *ParentF = I->getFunction();
-      if (VisitedFunctions.find(ParentF) != VisitedFunctions.end())
+      if (VisitedFunctions.count(ParentF))
         continue;
 
       if (ParentF->hasFnAttribute("referenced-indirectly")) {
-        return {true, {}};
+        return {};
       }
 
       if (ParentF->getCallingConv() == CallingConv::SPIR_KERNEL)
@@ -399,39 +398,30 @@ TraverseCGToFindSPIRKernels(const Function *StartingFunction) {
     }
   }
 
-  return {false, std::move(KernelNames)};
+  return std::move(KernelNames);
 }
 
 std::vector<StringRef> getKernelNamesUsingAssert(const Module &M) {
-  Optional<const Function *> DevicelibAssertFailFunction;
-  std::vector<StringRef> SPIRKernelNames;
-  // This loop finds all SPIR kernel's names and __devicelib_assert_fail
-  // function if it is present.
-  for (const Function &F : M) {
-    if (F.getCallingConv() == CallingConv::SPIR_KERNEL)
-      SPIRKernelNames.push_back(F.getName());
-
-    if (F.getName().startswith("__devicelib_assert_fail")) {
-      assert(!DevicelibAssertFailFunction.hasValue());
-      DevicelibAssertFailFunction = &F;
-    }
-  }
-
+  auto DevicelibAssertFailFunction = M.getFunction("__devicelib_assert_fail");
   if (!DevicelibAssertFailFunction)
     return {};
 
   auto TraverseResult =
-      TraverseCGToFindSPIRKernels(*DevicelibAssertFailFunction);
-  std::vector<StringRef> Result;
-  if (TraverseResult.first) {
-    // If assert is met in some indirectly callable function than
-    // we return all kernels in Module due to the current assert's design.
-    Result = std::move(SPIRKernelNames);
-  } else {
-    Result = std::move(TraverseResult.second);
+      TraverseCGToFindSPIRKernels(DevicelibAssertFailFunction);
+
+  if (TraverseResult.hasValue()) {
+    return std::move(*TraverseResult);
+  }
+
+  // Here we reached "referenced-indirectly", so we need to find all kernels and
+  // return them.
+  std::vector<StringRef> SPIRKernelNames;
+  for (const Function &F : M) {
+    if (F.getCallingConv() == CallingConv::SPIR_KERNEL)
+      SPIRKernelNames.push_back(F.getName());
   }
 
-  return Result;
+  return SPIRKernelNames;
 }
 
 // Gets reqd_work_group_size information for function Func.
