Rework logic to not add another argument to computeModuleProperties

Author: jzc

llvm/include/llvm/SYCLLowerIR/ComputeModuleRuntimeInfo.h

@@ -34,8 +34,7 @@ using EntryPointSet = SetVector<Function *>;
 
 PropSetRegTy computeModuleProperties(const Module &M,
                                      const EntryPointSet &EntryPoints,
-                                     const GlobalBinImageProps &GlobProps,
-                                     module_split::IRSplitMode SplitMode);
+                                     const GlobalBinImageProps &GlobProps);
 
 std::string computeModuleSymbolTable(const Module &M,
                                      const EntryPointSet &EntryPoints);

llvm/include/llvm/Support/PropertySetIO.h

@@ -230,6 +230,17 @@ class PropertySetRegistry {
     PropSet.insert({PropName, PropertyValue(PropVal)});
   }
 
+  void remove(StringRef Category, StringRef PropName) {
+    auto PropertySetIt = PropSetMap.find(Category);
+    if (PropertySetIt == PropSetMap.end())
+      return;
+    auto &PropertySet = PropertySetIt->second;
+    auto PropIt = PropertySet.find(PropName);
+    if (PropIt == PropertySet.end())
+      return;
+    PropertySet.erase(PropIt);
+  }
+
   /// Parses from the given \p Buf a property set registry.
   static Expected<std::unique_ptr<PropertySetRegistry>>
   read(const MemoryBuffer *Buf);

llvm/lib/SYCLLowerIR/ComputeModuleRuntimeInfo.cpp

@@ -152,8 +152,7 @@ std::optional<T> getKernelSingleEltMetadata(const Function &Func,
 
 PropSetRegTy computeModuleProperties(const Module &M,
                                      const EntryPointSet &EntryPoints,
-                                     const GlobalBinImageProps &GlobProps,
-                                     module_split::IRSplitMode SplitMode) {
+                                     const GlobalBinImageProps &GlobProps) {
 
   PropSetRegTy PropSet;
   {
@@ -162,16 +161,8 @@ PropSetRegTy computeModuleProperties(const Module &M,
     PropSet.add(PropSetRegTy::SYCL_DEVICELIB_REQ_MASK, RMEntry);
   }
   {
-    // Usually, we would only expect one ReqdWGSize, as the module passed to
-    // this function would be split according to that. However, when splitting
-    // is disabled, this cannot be guaranteed. In this case, we reset the value,
-    // which makes so that no value is reqd_work_group_size data is attached in
-    // in the device image.
-    SYCLDeviceRequirements DeviceReqs = computeDeviceRequirements(M, EntryPoints);
-    if (SplitMode == module_split::SPLIT_NONE)
-      DeviceReqs.ReqdWorkGroupSize.reset();
     PropSet.add(PropSetRegTy::SYCL_DEVICE_REQUIREMENTS,
-                DeviceReqs.asMap());
+                computeDeviceRequirements(M, EntryPoints).asMap());
   }
 
   // extract spec constant maps per each module

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

@@ -308,15 +308,24 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD,
                                  const GlobalBinImageProps &GlobProps, int I,
                                  StringRef Suff, StringRef Target = "") {
   auto PropSet =
-      computeModuleProperties(MD.getModule(), MD.entries(), GlobProps, SplitMode);
+      computeModuleProperties(MD.getModule(), MD.entries(), GlobProps);
+
+  // When the split mode is none, the required work group size will be added
+  // to the whole module, which will make the runtime unable to
+  // launch the other kernels in the module that have different
+  // required work group sizes or no requried work group sizes. So we need to
+  // remove the required work group size metadata in this case.
+  if (SplitMode == module_split::SPLIT_NONE)
+    PropSet.remove(PropSetRegTy::SYCL_DEVICE_REQUIREMENTS,
+                   "reqd_work_group_size_uint64_t");
 
   std::string NewSuff = Suff.str();
   if (!Target.empty()) {
     PropSet.add(PropSetRegTy::SYCL_DEVICE_REQUIREMENTS, "compile_target",
                 Target);
     NewSuff += "_";
-    NewSuff += Target;
   }
+  NewSuff += Target;
 
   std::error_code EC;
   std::string SCFile = makeResultFileName(".prop", I, NewSuff);