- When doing nested split of a module, the entry point is now limited to

  the entry points of the source module. This fixes
  SYCL/SpecConstants/2020/non_native/multiple-targets.cpp test failure.
- Make internal interfaces more convenient and consistent by passing
  ModuleDesc instead of Module + entry points + properties.
- Fix call graph traversal by detecting cases when a CallInst uses a
  function not as a callee (call target), but otherwise.

Author: kbobrovs

llvm/lib/SYCLLowerIR/ESIMD/ESIMDUtils.cpp

@@ -22,14 +22,15 @@ void traverseCallgraphUp(llvm::Function *F, CallGraphNodeAction ActionF,
     // Update all callers as well.
     for (auto It = CurF->use_begin(); It != CurF->use_end(); It++) {
       auto FCall = It->getUser();
+      auto ErrMsg =
+          llvm::Twine(__FILE__ " ") +
+          "Function use other than call detected while traversing call\n"
+          "graph up to a kernel";
       if (!isa<CallInst>(FCall)) {
         // A use other than a call is met...
         if (ErrorOnNonCallUse) {
           // ... non-call is an error - report
-          llvm::report_fatal_error(
-              llvm::Twine(__FILE__ " ") +
-              "Function use other than call detected while traversing call\n"
-              "graph up to a kernel");
+          llvm::report_fatal_error(ErrMsg);
         } else {
           // ... non-call is OK - add using function to the worklist
           if (auto *I = dyn_cast<Instruction>(FCall)) {
@@ -41,10 +42,17 @@ void traverseCallgraphUp(llvm::Function *F, CallGraphNodeAction ActionF,
           }
         }
       } else {
-        auto FCaller = cast<CallInst>(FCall)->getFunction();
+        auto *CI = cast<CallInst>(FCall);
 
-        if (!FunctionsVisited.count(FCaller)) {
-          Worklist.push_back(FCaller);
+        if ((CI->getCalledFunction() != CurF) && ErrorOnNonCallUse) {
+          // CurF is used in a call, but not as the callee.
+          llvm::report_fatal_error(ErrMsg);
+        } else {
+          auto FCaller = CI->getFunction();
+
+          if (!FunctionsVisited.count(FCaller)) {
+            Worklist.push_back(FCaller);
+          }
         }
       }
     }

llvm/tools/sycl-post-link/ModuleSplitter.cpp

@@ -135,35 +135,28 @@ bool isESIMDFunction(const Function &F) {
 
 // This function makes one or two groups depending on kernel types (SYCL, ESIMD)
 EntryPointGroupVec
-groupEntryPointsByKernelType(const Module &M, bool EmitOnlyKernelsAsEntryPoints,
-                             EntryPointVec *AllowedEntriesVec,
-                             EntryPointGroup::Properties BlueprintProps) {
-  SmallPtrSet<const Function *, 32> AllowedEntries;
-
-  if (AllowedEntriesVec) {
-    std::copy(AllowedEntriesVec->begin(), AllowedEntriesVec->end(),
-              std::inserter(AllowedEntries, AllowedEntries.end()));
-  }
+groupEntryPointsByKernelType(const ModuleDesc &MD,
+                             bool EmitOnlyKernelsAsEntryPoints) {
+  const Module &M = MD.getModule();
   EntryPointGroupVec EntryPointGroups{};
-  std::map<StringRef, EntryPointVec> EntryPointMap;
+  std::map<StringRef, EntryPointSet> EntryPointMap;
 
   // Only process module entry points:
   for (const auto &F : M.functions()) {
-    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints))
-      continue;
-    if (AllowedEntriesVec && (AllowedEntries.find(&F) == AllowedEntries.end()))
+    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints) ||
+        !MD.isEntryPointCandidate(F))
       continue;
 
     if (isESIMDFunction(F))
-      EntryPointMap[ESIMD_SCOPE_NAME].push_back(&F);
+      EntryPointMap[ESIMD_SCOPE_NAME].insert(&F);
     else
-      EntryPointMap[SYCL_SCOPE_NAME].push_back(&F);
+      EntryPointMap[SYCL_SCOPE_NAME].insert(&F);
   }
 
   if (!EntryPointMap.empty()) {
     for (auto &EPG : EntryPointMap) {
-      EntryPointGroups.emplace_back(
-          EntryPointGroup{EPG.first, std::move(EPG.second), BlueprintProps});
+      EntryPointGroups.emplace_back(EntryPointGroup{
+          EPG.first, std::move(EPG.second), MD.getEntryPointGroup().Props});
       EntryPointGroup &G = EntryPointGroups.back();
 
       if (G.GroupId == ESIMD_SCOPE_NAME) {
@@ -175,7 +168,8 @@ groupEntryPointsByKernelType(const Module &M, bool EmitOnlyKernelsAsEntryPoints,
     }
   } else {
     // No entry points met, record this.
-    EntryPointGroups.emplace_back(EntryPointGroup{SYCL_SCOPE_NAME, {}});
+    EntryPointGroups.emplace_back(
+        EntryPointGroup{SYCL_SCOPE_NAME, EntryPointSet{}});
     EntryPointGroup &G = EntryPointGroups.back();
     G.Props.HasESIMD = SyclEsimdSplitStatus::SYCL_ONLY;
   }
@@ -189,22 +183,23 @@ groupEntryPointsByKernelType(const Module &M, bool EmitOnlyKernelsAsEntryPoints,
 // which contains pairs of group id and entry points for that group. Each such
 // group along with IR it depends on (globals, functions from its call graph,
 // ...) will constitute a separate module.
-EntryPointGroupVec
-groupEntryPointsByScope(const Module &M, EntryPointsGroupScope EntryScope,
-                        bool EmitOnlyKernelsAsEntryPoints,
-                        EntryPointGroup::Properties BlueprintProps) {
+EntryPointGroupVec groupEntryPointsByScope(const ModuleDesc &MD,
+                                           EntryPointsGroupScope EntryScope,
+                                           bool EmitOnlyKernelsAsEntryPoints) {
   EntryPointGroupVec EntryPointGroups{};
   // Use MapVector for deterministic order of traversal (helps tests).
-  MapVector<StringRef, EntryPointVec> EntryPointMap;
+  MapVector<StringRef, EntryPointSet> EntryPointMap;
+  const Module &M = MD.getModule();
 
   // Only process module entry points:
   for (const auto &F : M.functions()) {
-    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints))
+    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints) ||
+        !MD.isEntryPointCandidate(F))
       continue;
 
     switch (EntryScope) {
     case Scope_PerKernel:
-      EntryPointMap[F.getName()].push_back(&F);
+      EntryPointMap[F.getName()].insert(&F);
       break;
 
     case Scope_PerModule: {
@@ -218,55 +213,59 @@ groupEntryPointsByScope(const Module &M, EntryPointsGroupScope EntryScope,
 
       Attribute Id = F.getFnAttribute(ATTR_SYCL_MODULE_ID);
       StringRef Val = Id.getValueAsString();
-      EntryPointMap[Val].push_back(&F);
+      EntryPointMap[Val].insert(&F);
       break;
     }
 
     case Scope_Global:
       // the map key is not significant here
-      EntryPointMap[GLOBAL_SCOPE_NAME].push_back(&F);
+      EntryPointMap[GLOBAL_SCOPE_NAME].insert(&F);
       break;
     }
   }
 
   if (!EntryPointMap.empty()) {
     EntryPointGroups.reserve(EntryPointMap.size());
     for (auto &EPG : EntryPointMap) {
-      EntryPointGroups.emplace_back(
-          EntryPointGroup{EPG.first, std::move(EPG.second), BlueprintProps});
+      EntryPointGroups.emplace_back(EntryPointGroup{
+          EPG.first, std::move(EPG.second), MD.getEntryPointGroup().Props});
       EntryPointGroup &G = EntryPointGroups.back();
       G.Props.Scope = EntryScope;
     }
   } else {
     // No entry points met, record this.
-    EntryPointGroups.push_back({GLOBAL_SCOPE_NAME, {}});
+    EntryPointGroups.emplace_back(
+        EntryPointGroup{GLOBAL_SCOPE_NAME, EntryPointSet{}});
   }
   return EntryPointGroups;
 }
 
 template <class EntryPoinGroupFunc>
-EntryPointGroupVec groupEntryPointsByAttribute(
-    const Module &M, StringRef AttrName, bool EmitOnlyKernelsAsEntryPoints,
-    EntryPoinGroupFunc F, EntryPointGroup::Properties BlueprintProps) {
+EntryPointGroupVec
+groupEntryPointsByAttribute(const ModuleDesc &MD, StringRef AttrName,
+                            bool EmitOnlyKernelsAsEntryPoints,
+                            EntryPoinGroupFunc F) {
   EntryPointGroupVec EntryPointGroups{};
-  std::map<StringRef, EntryPointVec> EntryPointMap;
+  std::map<StringRef, EntryPointSet> EntryPointMap;
+  const Module &M = MD.getModule();
 
   // Only process module entry points:
   for (const auto &F : M.functions()) {
-    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints)) {
+    if (!isEntryPoint(F, EmitOnlyKernelsAsEntryPoints) ||
+        !MD.isEntryPointCandidate(F)) {
       continue;
     }
     if (F.hasFnAttribute(AttrName)) {
-      EntryPointMap[AttrName].push_back(&F);
+      EntryPointMap[AttrName].insert(&F);
     } else {
-      EntryPointMap[""].push_back(&F);
+      EntryPointMap[""].insert(&F);
     }
   }
   if (!EntryPointMap.empty()) {
     EntryPointGroups.reserve(EntryPointMap.size());
     for (auto &EPG : EntryPointMap) {
-      EntryPointGroups.emplace_back(
-          EntryPointGroup{EPG.first, std::move(EPG.second), BlueprintProps});
+      EntryPointGroups.emplace_back(EntryPointGroup{
+          EPG.first, std::move(EPG.second), MD.getEntryPointGroup().Props});
       F(EntryPointGroups.back());
     }
   } else {
@@ -354,12 +353,11 @@ ModuleDesc extractSubModule(const ModuleDesc &MD,
   std::unique_ptr<Module> SubM = CloneModule(
       M, VMap, [&](const GlobalValue *GV) { return GVs.count(GV); });
   // Replace entry points with cloned ones.
-  EntryPointVec NewEPs;
-  const EntryPointVec &EPs = ModuleEntryPoints.Functions;
-  NewEPs.reserve(EPs.size());
-  std::transform(
-      EPs.cbegin(), EPs.cend(), std::inserter(NewEPs, NewEPs.end()),
-      [&VMap](const Function *F) { return cast<Function>(VMap[F]); });
+  EntryPointSet NewEPs;
+  const EntryPointSet &EPs = ModuleEntryPoints.Functions;
+  std::for_each(EPs.begin(), EPs.end(), [&](const Function *F) {
+    NewEPs.insert(cast<Function>(VMap[F]));
+  });
   ModuleEntryPoints.Functions = std::move(NewEPs);
   return ModuleDesc{std::move(SubM), std::move(ModuleEntryPoints), MD.Props};
 }
@@ -416,11 +414,9 @@ namespace llvm {
 namespace module_split {
 
 std::unique_ptr<ModuleSplitterBase>
-getSplitterByKernelType(ModuleDesc &&MD, bool EmitOnlyKernelsAsEntryPoints,
-                        EntryPointVec *AllowedEntries) {
-  EntryPointGroupVec Groups = groupEntryPointsByKernelType(
-      MD.getModule(), EmitOnlyKernelsAsEntryPoints, AllowedEntries,
-      MD.getEntryPointGroup().Props);
+getSplitterByKernelType(ModuleDesc &&MD, bool EmitOnlyKernelsAsEntryPoints) {
+  EntryPointGroupVec Groups =
+      groupEntryPointsByKernelType(MD, EmitOnlyKernelsAsEntryPoints);
   bool DoSplit = (Groups.size() > 1);
 
   if (DoSplit)
@@ -435,9 +431,8 @@ getSplitterByMode(ModuleDesc &&MD, IRSplitMode Mode,
                   bool EmitOnlyKernelsAsEntryPoints) {
   EntryPointsGroupScope Scope =
       selectDeviceCodeGroupScope(MD.getModule(), Mode, AutoSplitIsGlobalScope);
-  EntryPointGroupVec Groups = groupEntryPointsByScope(
-      MD.getModule(), Scope, EmitOnlyKernelsAsEntryPoints,
-      MD.getEntryPointGroup().Props);
+  EntryPointGroupVec Groups =
+      groupEntryPointsByScope(MD, Scope, EmitOnlyKernelsAsEntryPoints);
   assert(!Groups.empty() && "At least one group is expected");
   bool DoSplit = (Mode != SPLIT_NONE &&
                   (Groups.size() > 1 || !Groups.cbegin()->Functions.empty()));
@@ -526,7 +521,7 @@ void tab(int N) {
   }
 }
 
-void dumpEntryPoints(const EntryPointVec &C, const char *msg, int Tab) {
+void dumpEntryPoints(const EntryPointSet &C, const char *msg, int Tab) {
   tab(Tab);
   llvm::errs() << "ENTRY POINTS"
                << " " << msg << " {\n";
@@ -565,8 +560,8 @@ void ModuleDesc::renameDuplicatesOf(const Module &MA, StringRef Suff) {
   Module &MB = getModule();
 #ifndef NDEBUG
   DenseSet<StringRef> EntryNamesB;
-  auto It0 = entries().cbegin();
-  auto It1 = entries().cend();
+  const auto It0 = entries().begin();
+  const auto It1 = entries().end();
   std::for_each(It0, It1,
                 [&](const Function *F) { EntryNamesB.insert(F->getName()); });
 #endif // NDEBUG
@@ -640,52 +635,47 @@ void ModuleDesc::dump() const {
 
 void EntryPointGroup::saveNames(std::vector<std::string> &Dest) const {
   Dest.reserve(Dest.size() + Functions.size());
-  std::transform(Functions.cbegin(), Functions.cend(),
+  std::transform(Functions.begin(), Functions.end(),
                  std::inserter(Dest, Dest.end()),
                  [](const Function *F) { return F->getName().str(); });
 }
 
 void EntryPointGroup::rebuildFromNames(const std::vector<std::string> &Names,
                                        const Module &M) {
   Functions.clear();
-  Functions.reserve(Names.size());
   auto It0 = Names.cbegin();
   auto It1 = Names.cend();
-  std::transform(It0, It1, std::inserter(Functions, Functions.begin()),
-                 [&M](const std::string &Name) {
-                   const Function *F = M.getFunction(Name);
-                   assert(F && "entry point lost");
-                   return F;
-                 });
+  std::for_each(It0, It1, [&](const std::string &Name) {
+    const Function *F = M.getFunction(Name);
+    assert(F && "entry point lost");
+    Functions.insert(F);
+  });
 }
 
 void EntryPointGroup::rebuild(const Module &M) {
   if (Functions.size() == 0) {
     return;
   }
-  EntryPointVec NewFunctions;
-  NewFunctions.reserve(Functions.size());
-  auto It0 = Functions.cbegin();
-  auto It1 = Functions.cend();
-  std::transform(It0, It1, std::inserter(NewFunctions, NewFunctions.begin()),
-                 [&M](const Function *F) {
-                   Function *NewF = M.getFunction(F->getName());
-                   assert(NewF && "entry point lost");
-                   return NewF;
-                 });
+  EntryPointSet NewFunctions;
+  const auto It0 = Functions.begin();
+  const auto It1 = Functions.end();
+  std::for_each(It0, It1, [&](const Function *F) {
+    Function *NewF = M.getFunction(F->getName());
+    assert(NewF && "entry point lost");
+    NewFunctions.insert(NewF);
+  });
   Functions = std::move(NewFunctions);
 }
 
 std::unique_ptr<ModuleSplitterBase>
 getESIMDDoubleGRFSplitter(ModuleDesc &&MD, bool EmitOnlyKernelsAsEntryPoints) {
   EntryPointGroupVec Groups = groupEntryPointsByAttribute(
-      MD.getModule(), ATTR_DOUBLE_GRF, EmitOnlyKernelsAsEntryPoints,
+      MD, ATTR_DOUBLE_GRF, EmitOnlyKernelsAsEntryPoints,
       [](EntryPointGroup &G) {
         if (G.GroupId == ATTR_DOUBLE_GRF) {
           G.Props.UsesDoubleGRF = true;
         }
-      },
-      MD.getEntryPointGroup().Props);
+      });
   assert(!Groups.empty() && "At least one group is expected");
   assert(Groups.size() <= 2 && "At most 2 groups are expected");