Merge branches 'dummy-image-runtime' and 'dummy-images' into HEAD

Author: jzc

llvm/test/tools/sycl-post-link/virtual-functions/dummy.ll

@@ -0,0 +1,37 @@
+; RUN: sycl-post-link -split=auto -properties -S < %s -o %t.table
+; RUN: FileCheck %s --input-file=%t.table --check-prefix=CHECK-TABLE
+; RUN: FileCheck %s --input-file=%t_0.ll --check-prefix=CHECK-FP64-SPLIT
+; RUN: FileCheck %s --input-file=%t_1.ll --check-prefix=CHECK-FP64-DUMMY
+; RUN: FileCheck %s --input-file=%t_1.prop --check-prefix=CHECK-FP64-DUMMY-PROPS
+; RUN: FileCheck %s --input-file=%t_2.ll --check-prefix=CHECK-FP32-SPLIT
+
+; CHECK-TABLE:      _0.prop
+; CHECK-TABLE-NEXT: _1.prop
+; CHECK-TABLE-NEXT: _2.prop
+
+; CHECK-FP64-SPLIT: define spir_func void @bar()
+; CHECK-FP32-SPLIT: define spir_func void @foo()
+
+; CHECK-FP64-DUMMY: define spir_func void @bar()
+; CHECK-FP64-DUMMY-NEXT: entry:
+; CHECK-FP64-DUMMY-NEXT: ret void
+
+; CHECK-FP64-DUMMY-PROPS: dummy=1
+
+define spir_func void @foo() #1 {
+  %x = alloca float
+  ret void
+}
+
+define spir_func void @bar() #1 !sycl_used_aspects !1 {
+  %x = alloca double
+  %d = load double, ptr %x
+  %res = fadd double %d, %d
+  ret void
+}
+
+attributes #1 = { "sycl-module-id"="v.cpp" "indirectly-callable"="setA" }
+
+!sycl_aspects = !{!0}
+!0 = !{!"fp64", i32 6}
+!1 = !{i32 6}

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

@@ -305,7 +305,8 @@ std::string saveModuleIR(Module &M, int I, StringRef Suff) {
 
 std::string saveModuleProperties(module_split::ModuleDesc &MD,
                                  const GlobalBinImageProps &GlobProps, int I,
-                                 StringRef Suff, StringRef Target = "") {
+                                 StringRef Suff, StringRef Target = "",
+                                 bool IsDummy = false) {
   auto PropSet =
       computeModuleProperties(MD.getModule(), MD.entries(), GlobProps);
 
@@ -317,6 +318,10 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD,
     NewSuff += Target;
   }
 
+  if (IsDummy) {
+    PropSet.add(PropSetRegTy::SYCL_VIRTUAL_FUNCTIONS, "dummy", 1);
+  }
+
   std::error_code EC;
   std::string SCFile = makeResultFileName(".prop", I, NewSuff);
   raw_fd_ostream SCOut(SCFile, EC);
@@ -415,7 +420,8 @@ void addTableRow(util::SimpleTable &Table,
 //   IR component saving is skipped, and this file name is recorded as such in
 //   the result.
 void saveModule(std::vector<std::unique_ptr<util::SimpleTable>> &OutTables,
-                module_split::ModuleDesc &MD, int I, StringRef IRFilename) {
+                module_split::ModuleDesc &MD, int I, StringRef IRFilename,
+                bool IsDummy = false) {
   IrPropSymFilenameTriple BaseTriple;
   StringRef Suffix = getModuleSuffix(MD);
   MD.saveSplitInformationAsMetadata();
@@ -439,8 +445,8 @@ void saveModule(std::vector<std::unique_ptr<util::SimpleTable>> &OutTables,
       GlobalBinImageProps Props = {EmitKernelParamInfo, EmitProgramMetadata,
                                    EmitExportedSymbols, EmitImportedSymbols,
                                    DeviceGlobals};
-      CopyTriple.Prop =
-          saveModuleProperties(MD, Props, I, Suffix, OutputFile.Target);
+      CopyTriple.Prop = saveModuleProperties(MD, Props, I, Suffix,
+                                             OutputFile.Target, IsDummy);
     }
     addTableRow(*Table, CopyTriple);
   }
@@ -740,6 +746,36 @@ bool isTargetCompatibleWithModule(const std::string &Target,
   return true;
 }
 
+std::optional<module_split::ModuleDesc>
+makeDummy(module_split::ModuleDesc &MD) {
+  bool hasVirtualFunctions = false;
+  bool hasOptionalKernelFeatures = false;
+  for (Function &F : MD.getModule().functions()) {
+    if (F.hasFnAttribute("indirectly-callable"))
+      hasVirtualFunctions = true;
+    if (F.getMetadata("sycl_used_aspects"))
+      hasOptionalKernelFeatures = true;
+    if (hasVirtualFunctions && hasOptionalKernelFeatures)
+      break;
+  }
+  if (!hasVirtualFunctions || !hasOptionalKernelFeatures)
+    return {};
+
+  auto MDCopy = MD.clone();
+
+  for (Function &F : MDCopy.getModule().functions()) {
+    if (!F.hasFnAttribute("indirectly-callable"))
+      continue;
+
+    F.erase(F.begin(), F.end());
+    BasicBlock *newBB = BasicBlock::Create(F.getContext(), "entry", &F);
+    IRBuilder<> builder(newBB);
+    builder.CreateRetVoid();
+  }
+
+  return MDCopy;
+}
+
 std::vector<std::unique_ptr<util::SimpleTable>>
 processInputModule(std::unique_ptr<Module> M) {
   // Construct the resulting table which will accumulate all the outputs.
@@ -887,6 +923,16 @@ processInputModule(std::unique_ptr<Module> M) {
 
       ++ID;
     }
+
+    bool dummyEmitted = false;
+    for (module_split::ModuleDesc &IrMD : MMs) {
+      if (auto Dummy = makeDummy(IrMD)) {
+        saveModule(Tables, *Dummy, ID, OutIRFileName, /*IsDummy*/ true);
+        dummyEmitted = true;
+      }
+    }
+    if (dummyEmitted)
+      ++ID;
   }
   return Tables;
 }

sycl/source/detail/device_binary_image.cpp

@@ -52,7 +52,7 @@ std::ostream &operator<<(std::ostream &Out, const DeviceBinaryProperty &P) {
     break;
   }
   case SYCL_PROPERTY_TYPE_STRING:
-    Out << P.asCString();
+    Out << P.asStringView();
     break;
   default:
     assert(false && "Unsupported property");
@@ -77,14 +77,14 @@ ByteArray DeviceBinaryProperty::asByteArray() const {
   return {Data, Prop->ValSize};
 }
 
-const char *DeviceBinaryProperty::asCString() const {
+std::string_view DeviceBinaryProperty::asStringView() const {
   assert((Prop->Type == SYCL_PROPERTY_TYPE_STRING ||
           Prop->Type == SYCL_PROPERTY_TYPE_BYTE_ARRAY) &&
          "property type mismatch");
   assert(Prop->ValSize > 0 && "property size mismatch");
   // Byte array stores its size in first 8 bytes
   size_t Shift = Prop->Type == SYCL_PROPERTY_TYPE_BYTE_ARRAY ? 8 : 0;
-  return ur::cast<const char *>(Prop->ValAddr) + Shift;
+  return {ur::cast<const char *>(Prop->ValAddr) + Shift, Prop->ValSize};
 }
 
 void RTDeviceBinaryImage::PropertyRange::init(sycl_device_binary Bin,

sycl/source/detail/device_binary_image.hpp

@@ -73,7 +73,7 @@ class DeviceBinaryProperty {
 
   uint32_t asUint32() const;
   ByteArray asByteArray() const;
-  const char *asCString() const;
+  std::string_view asStringView() const;
 
 protected:
   friend std::ostream &operator<<(std::ostream &Out,

sycl/source/detail/program_manager/program_manager.cpp

@@ -676,10 +676,11 @@ ProgramManager::collectDependentDeviceImagesForVirtualFunctions(
   std::set<std::string> HandledSets;
   std::queue<std::string> WorkList;
   for (const sycl_device_binary_property &VFProp : Img.getVirtualFunctions()) {
-    std::string StrValue = DeviceBinaryProperty(VFProp).asCString();
+    std::string_view StrValue = DeviceBinaryProperty(VFProp).asStringView();
     // Device image passed to this function is expected to contain SYCL kernels
     // and therefore it may only use virtual function sets, but cannot provide
-    // them. We expect to see just a single property here
+    // them. Additionally, it cannot be a dummy image.
+    // We expect to see just a single property here
     assert(std::string(VFProp->Name) == "uses-virtual-functions-set" &&
            "Unexpected virtual function property");
     for (const auto &SetName : detail::split_string(StrValue, ',')) {
@@ -700,22 +701,38 @@ ProgramManager::collectDependentDeviceImagesForVirtualFunctions(
       // virtual-functions-set properties, but their handling is the same: we
       // just grab all sets they reference and add them for consideration if
       // we haven't done so already.
+      bool isDummyImage = false;
       for (const sycl_device_binary_property &VFProp :
            BinImage->getVirtualFunctions()) {
-        std::string StrValue = DeviceBinaryProperty(VFProp).asCString();
+        if (VFProp->Name == std::string_view("dummy-image")) {
+          isDummyImage = true;
+          continue;
+        }
+        std::string_view StrValue = DeviceBinaryProperty(VFProp).asStringView();
         for (const auto &SetName : detail::split_string(StrValue, ',')) {
           if (HandledSets.insert(SetName).second)
             WorkList.push(SetName);
         }
       }
 
-      // TODO: Complete this part about handling of incompatible device images.
       // If device image uses the same virtual function set, then we only
       // link it if it is compatible.
       // However, if device image provides virtual function set and it is
       // incompatible, then we should link its "dummy" version to avoid link
       // errors about unresolved external symbols.
-      if (doesDevSupportDeviceRequirements(Dev, *BinImage))
+      // Note: we only link when exactly one of
+      // doesDevSupportDeviceRequirements(Dev, *BinImage) and
+      // isDummyImage is true. We don't want to link every dummy image,
+      // otherwise we could run into linking errors defining the same symbol
+      // multiple times. For every image providing virtual functions that has
+      // a dummy image, the dummy image will have the same device requirements
+      // as the original image. So when the dummy image does support the
+      // device requirements, we know that the corresponding image providing
+      // actual definitions will be linked and not the dummy. And vice versa:
+      // when the dummy image does not support the device requirements, we
+      // know the corresponding image providing virtual functions was not
+      // linked and we must link the dummy image.
+      if (doesDevSupportDeviceRequirements(Dev, *BinImage) + isDummyImage == 1)
         DeviceImagesToLink.insert(BinImage);
     }
   }
@@ -1773,7 +1790,9 @@ void ProgramManager::addImages(sycl_device_binaries DeviceBinary) {
     // Record mapping between virtual function sets and device images
     for (const sycl_device_binary_property &VFProp :
          Img->getVirtualFunctions()) {
-      std::string StrValue = DeviceBinaryProperty(VFProp).asCString();
+      if (VFProp->Name == std::string_view("dummy-image"))
+        continue;
+      std::string_view StrValue = DeviceBinaryProperty(VFProp).asStringView();
       for (const auto &SetName : detail::split_string(StrValue, ','))
         m_VFSet2BinImage[SetName].insert(Img.get());
     }