[SYCL] Treat only SYCL kernels as entry points for FPGA target (#4693)

AlexeySachkov · web-flow · commit 41676bcec4b1 · 2021-10-06T09:50:07.000+03:00
Introduced new `sycl-post-link` option `-emit-only-kernels-as-entry-points`,
which allows to stop treating `SYCL_EXTERNAL` functions as entry points.

Updated clang driver to pass the new option for `spir64_fpga` target, because
on FPGA, there are no use cases for preserving `SYCL_EXTERNAL` functions
as separate entries in device images if they are not referenced and it only
causes compilation slowdowns due to increased amount of device code.
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8916,6 +8916,13 @@ void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,
     // auto is the default split mode
     addArgs(CmdArgs, TCArgs, {"-split=auto"});
   }
+
+  // On FPGA target we don't need non-kernel functions as entry points, because
+  // it only increases amount of code for device compiler to handle, without any
+  // actual benefits.
+  if (getToolChain().getTriple().getArchName() == "spir64_fpga")
+    addArgs(CmdArgs, TCArgs, {"-emit-only-kernels-as-entry-points"});
+
   // OPT_fsycl_device_code_split is not checked as it is an alias to
   // -fsycl-device-code-split=auto
 
diff --git a/clang/test/Driver/sycl-offload-intelfpga.cpp b/clang/test/Driver/sycl-offload-intelfpga.cpp
@@ -29,6 +29,24 @@
 // CHK-RANGE-ROUNDING: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-disable-range-rounding"
 // CHK-RANGE-ROUNDING: clang{{.*}} "-fsycl-disable-range-rounding"{{.*}} "-fsycl-is-host"
 
+/// FPGA target implies -emit-only-kernels-as-entry-points in sycl-post-link
+// RUN:   %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fintelfpga %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-NON-KERNEL-ENTRY-POINTS %s
+// RUN:   %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_fpga-unknown-unknown %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-NON-KERNEL-ENTRY-POINTS %s
+// CHK-NON-KERNEL-ENTRY-POINTS: sycl-post-link{{.*}} "-emit-only-kernels-as-entry-points"
+
+/// Non-FPGA targets should not imply -emit-only-kernels-as-entry-points in sycl-post-link
+// RUN:   %clang -### -fsycl -fsycl-targets=spir64_fpga,spir64_gen %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHK-NON-KERNEL-ENTRY-POINTS-NEG-1
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-1: clang{{.*}} "-triple" "spir64_fpga-unknown-unknown"
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-1: sycl-post-link{{.*}} "-emit-only-kernels-as-entry-points"
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-1: clang{{.*}} "-triple" "spir64_gen-unknown-unknown"
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-1: sycl-post-link
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-1-NOT: "-emit-only-kernels-as-entry-points"
+// RUN:   %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=CHK-NON-KERNEL-ENTRY-POINTS-NEG-2
+// CHK-NON-KERNEL-ENTRY-POINTS-NEG-2-NOT: "-emit-only-kernels-as-entry-points"
+
 /// -fsycl-disable-range-rounding is applied to all compilations if fpga is used
 // RUN:   %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_fpga-unknown-unknown,spir64_gen-unknown-unknown %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-RANGE-ROUNDING-MULTI %s
diff --git a/llvm/test/tools/sycl-post-link/spec-constants/spec_const_and_split.ll b/llvm/test/tools/sycl-post-link/spec-constants/spec_const_and_split.ll
@@ -19,27 +19,25 @@
 
 declare dso_local spir_func zeroext i1 @_Z33__sycl_getScalarSpecConstantValueIbET_PKc(i8 addrspace(4)*)
 
-define dso_local spir_kernel void @KERNEL_AAA() #0 {
+define dso_local spir_kernel void @KERNEL_AAA() {
   %1 = call spir_func zeroext i1 @_Z33__sycl_getScalarSpecConstantValueIbET_PKc(i8 addrspace(4)* addrspacecast (i8* getelementptr inbounds ([10 x i8], [10 x i8]* @SCSymID, i64 0, i64 0) to i8 addrspace(4)*))
 ; CHECK-IR0: %{{[0-9]+}} = call i1 @_Z20__spirv_SpecConstantib(i32 1, i1 false)
   %2 = call spir_func zeroext i1 @_Z33__sycl_getScalarSpecConstantValueIbET_PKc(i8 addrspace(4)* addrspacecast (i8* getelementptr inbounds ([11 x i8], [11 x i8]* @SCSymID2, i64 0, i64 0) to i8 addrspace(4)*))
 ; CHECK-IR0: %{{[0-9]+}} = call i1 @_Z20__spirv_SpecConstantib(i32 0, i1 false)
   ret void
 }
 
-define dso_local spir_kernel void @KERNEL_BBB() #0 {
+define dso_local spir_kernel void @KERNEL_BBB() {
   %1 = call spir_func zeroext i1 @_Z33__sycl_getScalarSpecConstantValueIbET_PKc(i8 addrspace(4)* addrspacecast (i8* getelementptr inbounds ([10 x i8], [10 x i8]* @SCSymID, i64 0, i64 0) to i8 addrspace(4)*))
 ; CHECK-IR1: %{{[0-9]+}} = call i1 @_Z20__spirv_SpecConstantib(i32 0, i1 false)
   ret void
 }
 
-define dso_local spir_kernel void @KERNEL_CCC() #0 {
+define dso_local spir_kernel void @KERNEL_CCC() {
 ; CHECK-IR2: define{{.*}}spir_kernel void @KERNEL_CCC
   ret void
 }
 
-attributes #0 = { "sycl-module-id"="a.cpp" }
-
 ; CHECK-IR0: !sycl.specialization-constants = !{![[#MD0:]], ![[#MD1:]]}
 ; CHECK-IR0: ![[#MD0:]] = !{!"SpecConst2", i32 0, i32 0, i32 1}
 ; CHECK-IR0: ![[#MD1:]] = !{!"SpecConst", i32 1, i32 0, i32 1}
diff --git a/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-global.ll b/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-global.ll
@@ -1,9 +1,12 @@
+; This test checks handling of unreferenced functions with sycl-module-id
+; attribute with splitting in global mode.
+
 ; RUN: sycl-post-link -ir-output-only -split=auto -S %s -o %t.ll
-; RUN: FileCheck %s -input-file=%t.ll
+; RUN: FileCheck %s -input-file=%t.ll --check-prefix=CHECK-ALL
+
+; RUN: sycl-post-link -ir-output-only -emit-only-kernels-as-entry-points -split=auto -S %s -o %t.ll
+; RUN: FileCheck %s -input-file=%t.ll --check-prefix=CHECK-KERNEL-ONLY --implicit-check-not @externalDeviceFunc
 
-; This test checks that unreferenced functions with sycl-module-id
-; attribute are not dropped from the module after splitting
-; in global mode.
 
 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"
@@ -18,5 +21,7 @@ define dso_local spir_kernel void @kernel1() #0 {
 
 attributes #0 = { "sycl-module-id"="a.cpp" }
 
-; CHECK-DAG: define dso_local spir_func void @externalDeviceFunc()
-; CHECK-DAG: define dso_local spir_kernel void @kernel1()
+; CHECK-ALL-DAG: define dso_local spir_func void @externalDeviceFunc()
+; CHECK-ALL-DAG: define dso_local spir_kernel void @kernel1()
+;
+; CHECK-KERNEL-ONLY: define dso_local spir_kernel void @kernel1()
diff --git a/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-kernel.ll b/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-kernel.ll
@@ -1,12 +1,25 @@
+; This test checks handling of unreferenced functions with sycl-module-id
+; attribute with splitting in per-kernel mode.
+
 ; RUN: sycl-post-link -split=kernel -symbols -S %s -o %t.table
+; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-IR0
+; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-IR1
+; RUN: FileCheck %s -input-file=%t_2.ll --check-prefixes CHECK-IR2
+; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM0
+; RUN: FileCheck %s -input-file=%t_1.sym --check-prefixes CHECK-SYM1
+; RUN: FileCheck %s -input-file=%t_2.sym --check-prefixes CHECK-SYM2
+
+; RUN: sycl-post-link -split=kernel -emit-only-kernels-as-entry-points -symbols -S %s -o %t.table
 ; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-IR1
 ; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-IR2
 ; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM1
 ; RUN: FileCheck %s -input-file=%t_1.sym --check-prefixes CHECK-SYM2
+; RUN: FileCheck %s -input-file=%t.table --check-prefixes CHECK-TABLE
 
-; This test checks that unreferenced functions with sycl-module-id
-; attribute are not dropped from the module after splitting
-; in per-kernel mode.
+; CHECK-TABLE: [Code|Properties|Symbols]
+; CHECK-TABLE-NEXT: {{.*}}_0.ll|{{.*}}_0.prop|{{.*}}_0.sym
+; CHECK-TABLE-NEXT: {{.*}}_1.ll|{{.*}}_1.prop|{{.*}}_1.sym
+; CHECK-TABLE-EMPTY:
 
 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"
@@ -19,10 +32,19 @@ define dso_local spir_kernel void @kernel1() #0 {
   ret void
 }
 
+define dso_local spir_kernel void @kernel2() #0 {
+  ret void
+}
+
 attributes #0 = { "sycl-module-id"="a.cpp" }
 
-; CHECK-IR1: define dso_local spir_func void @externalDeviceFunc()
-; CHECK-IR2: define dso_local spir_kernel void @kernel1()
+; CHECK-IR0: define dso_local spir_func void @externalDeviceFunc()
+; CHECK-IR1: define dso_local spir_kernel void @kernel1()
+; CHECK-IR2: define dso_local spir_kernel void @kernel2()
 
-; CHECK-SYM1: externalDeviceFunc
-; CHECK-SYM2: kernel1
+; CHECK-SYM0: externalDeviceFunc
+; CHECK-SYM0-EMPTY:
+; CHECK-SYM1: kernel1
+; CHECK-SYM1-EMPTY:
+; CHECK-SYM2: kernel2
+; CHECK-SYM2-EMPTY:
diff --git a/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-source1.ll b/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-source1.ll
@@ -1,12 +1,19 @@
+; This test checks handling of unreferenced functions with sycl-module-id
+; attribute with splitting in per-source mode.
+
 ; RUN: sycl-post-link -split=source -symbols -S %s -o %t.table
 ; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-IR1
 ; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-IR2
 ; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM1
 ; RUN: FileCheck %s -input-file=%t_1.sym --check-prefixes CHECK-SYM2
 
-; This test checks that unreferenced functions with sycl-module-id
-; attribute are not dropped from the module after splitting
-; in per-source mode.
+; RUN: sycl-post-link -split=source -emit-only-kernels-as-entry-points -symbols -S %s -o %t.table
+; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM2
+; RUN: FileCheck %s -input-file=%t.table --check-prefixes CHECK-TABLE
+; CHECK-TABLE: [Code|Properties|Symbols]
+; CHECK-TABLE-NEXT: {{.*}}.ll|{{.*}}_0.prop|{{.*}}_0.sym
+; CHECK-TABLE-EMPTY:
+
 
 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"
@@ -26,4 +33,6 @@ attributes #1 = { "sycl-module-id"="b.cpp" }
 ; CHECK-IR2: define dso_local spir_kernel void @kernel1()
 
 ; CHECK-SYM1: externalDeviceFunc
+; CHECK-SYM1-EMPTY:
 ; CHECK-SYM2: kernel1
+; CHECK-SYM2-EMPTY:
diff --git a/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-source2.ll b/llvm/test/tools/sycl-post-link/sycl-external-funcs/split-per-source2.ll
@@ -1,11 +1,19 @@
+; This test checks handling of referenced SYCL_EXTERNAL functions with
+; sycl-module-id attribute with splitting in per-source mode.
+
 ; RUN: sycl-post-link -split=source -symbols -S %s -o %t.table
 ; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-IR1
 ; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-IR2
 ; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM1
 ; RUN: FileCheck %s -input-file=%t_1.sym --check-prefixes CHECK-SYM2
 
-; This test checks that the definition of function externalDeviceFunc is
-; present in both resulting modules when per-source split is requested.
+; RUN: sycl-post-link -split=source -emit-only-kernels-as-entry-points -symbols -S %s -o %t.table
+; RUN: FileCheck %s -input-file=%t_0.sym --check-prefixes CHECK-SYM2
+; RUN: FileCheck %s -input-file=%t.table --check-prefixes CHECK-TABLE
+; CHECK-TABLE: [Code|Properties|Symbols]
+; CHECK-TABLE-NEXT: {{.*}}.ll|{{.*}}_0.prop|{{.*}}_0.sym
+; CHECK-TABLE-EMPTY:
+
 
 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"
@@ -27,4 +35,6 @@ attributes #1 = { "sycl-module-id"="b.cpp" }
 ; CHECK-IR2: define dso_local spir_kernel void @kernel1()
 
 ; CHECK-SYM1: externalDeviceFunc
+; CHECK-SYM1-EMPTY:
 ; CHECK-SYM2: kernel1
+; CHECK-SYM2-EMPTY:
diff --git a/llvm/test/tools/sycl-post-link/sym_but_no_split.ll b/llvm/test/tools/sycl-post-link/sym_but_no_split.ll
@@ -6,22 +6,20 @@
 ; RUN: FileCheck %s -input-file=%t.files.table --check-prefixes CHECK-TABLE
 ; RUN: FileCheck %s -input-file=%t.files_0.sym --match-full-lines --check-prefixes CHECK-SYM
 
-define dso_local spir_kernel void @KERNEL_AAA() #0 {
+define dso_local spir_kernel void @KERNEL_AAA() {
 ; CHECK-SYM-NOT: {{[a-zA-Z0-9._@]+}}
 ; CHECK-SYM: KERNEL_AAA
 entry:
   ret void
 }
 
-define dso_local spir_kernel void @KERNEL_BBB() #0 {
+define dso_local spir_kernel void @KERNEL_BBB() {
 ; CHECK-SYM-NEXT: KERNEL_BBB
 ; CHECK-SYM-EMPTY:
 entry:
   ret void
 }
 
-attributes #0 = { "sycl-module-id"="a.cpp" }
-
 ; CHECK-TABLE: [Code|Properties|Symbols]
 ; CHECK-TABLE-NEXT: {{.*}}files_0.sym
 ; CHECK-TABLE-EMPTY:
diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp
@@ -175,6 +175,12 @@ static cl::opt<bool> EmitExportedSymbols{"emit-exported-symbols",
                                          cl::desc("emit exported symbols"),
                                          cl::cat(PostLinkCat)};
 
+static cl::opt<bool> EmitOnlyKernelsAsEntryPoints{
+    "emit-only-kernels-as-entry-points",
+    cl::desc("Consider only sycl_kernel functions as entry points for "
+             "device code split"),
+    cl::cat(PostLinkCat), cl::init(false)};
+
 struct ImagePropSaveInfo {
   bool NeedDeviceLibReqMask;
   bool DoSpecConst;
@@ -261,6 +267,22 @@ static bool funcIsSpirvSyclBuiltin(StringRef FName) {
   return FName.startswith("__spirv_") || FName.startswith("__sycl_");
 }
 
+static bool isEntryPoint(const Function &F) {
+  // Kernels are always considered to be entry points
+  if (CallingConv::SPIR_KERNEL == F.getCallingConv())
+    return true;
+
+  if (!EmitOnlyKernelsAsEntryPoints) {
+    // If not disabled, SYCL_EXTERNAL functions with sycl-module-id attribute
+    // are also considered as entry points (except __spirv_* and __sycl_*
+    // functions)
+    return F.hasFnAttribute(ATTR_SYCL_MODULE_ID) &&
+           !funcIsSpirvSyclBuiltin(F.getName());
+  }
+
+  return false;
+}
+
 // This function decides how kernels of the input module M will be distributed
 // ("split") into multiple modules based on the command options and IR
 // attributes. The decision is recorded in the output map parameter
@@ -271,37 +293,34 @@ static void collectKernelModuleMap(
     Module &M, std::map<StringRef, std::vector<Function *>> &ResKernelModuleMap,
     KernelMapEntryScope EntryScope) {
 
-  // Process module entry points: kernels and SYCL_EXTERNAL functions.
-  // Only they have sycl-module-id attribute, so any other unrefenced
-  // functions are dropped. SPIRV and SYCL builtin functions are not
-  // considered as module entry points.
+  // Only process module entry points:
   for (auto &F : M.functions()) {
-    if (F.hasFnAttribute(ATTR_SYCL_MODULE_ID) &&
-        !funcIsSpirvSyclBuiltin(F.getName())) {
-      switch (EntryScope) {
-      case Scope_PerKernel:
-        ResKernelModuleMap[F.getName()].push_back(&F);
-        break;
-      case Scope_PerModule: {
-        Attribute Id = F.getFnAttribute(ATTR_SYCL_MODULE_ID);
-        StringRef Val = Id.getValueAsString();
-        ResKernelModuleMap[Val].push_back(&F);
-        break;
-      }
-      case Scope_Global:
-        // the map key is not significant here
-        ResKernelModuleMap[GLOBAL_SCOPE_NAME].push_back(&F);
-        break;
-      }
-    } else if (EntryScope == Scope_PerModule &&
-               F.getCallingConv() == CallingConv::SPIR_KERNEL) {
-      // TODO It may make sense to group all kernels w/o the attribute into
-      // a separate module rather than issuing an error. Should probably be
-      // controlled by an option.
-      // Functions with spir_func calling convention are allowed to not have
-      // a sycl-module-id attribute.
-      error("no '" + Twine(ATTR_SYCL_MODULE_ID) + "' attribute in kernel '" +
-            F.getName() + "', per-module split not possible");
+    if (!isEntryPoint(F))
+      continue;
+
+    switch (EntryScope) {
+    case Scope_PerKernel:
+      ResKernelModuleMap[F.getName()].push_back(&F);
+      break;
+    case Scope_PerModule: {
+      if (!F.hasFnAttribute(ATTR_SYCL_MODULE_ID))
+        // TODO It may make sense to group all kernels w/o the attribute into
+        // a separate module rather than issuing an error. Should probably be
+        // controlled by an option.
+        // Functions with spir_func calling convention are allowed to not have
+        // a sycl-module-id attribute.
+        error("no '" + Twine(ATTR_SYCL_MODULE_ID) + "' attribute in kernel '" +
+              F.getName() + "', per-module split not possible");
+
+      Attribute Id = F.getFnAttribute(ATTR_SYCL_MODULE_ID);
+      StringRef Val = Id.getValueAsString();
+      ResKernelModuleMap[Val].push_back(&F);
+      break;
+    }
+    case Scope_Global:
+      // the map key is not significant here
+      ResKernelModuleMap["<GLOBAL>"].push_back(&F);
+      break;
     }
   }
 }
@@ -872,13 +891,9 @@ static ModulePair splitSyclEsimd(std::unique_ptr<Module> M) {
   std::vector<Function *> SyclFunctions;
   std::vector<Function *> EsimdFunctions;
   // Collect information about the SYCL and ESIMD functions in the module.
-  // Process module entry points: kernels and SYCL_EXTERNAL functions.
-  // Only they have sycl-module-id attribute, so any other unrefenced
-  // functions are dropped. SPIRV and SYCL builtin functions are not
-  // considered as module entry points.
+  // Only process module entry points.
   for (auto &F : M->functions()) {
-    if (F.hasFnAttribute(ATTR_SYCL_MODULE_ID) &&
-        !funcIsSpirvSyclBuiltin(F.getName())) {
+    if (isEntryPoint(F)) {
       if (F.getMetadata("sycl_explicit_simd"))
         EsimdFunctions.push_back(&F);
       else
@@ -950,8 +965,8 @@ int main(int argc, char **argv) {
       "- SYCL and ESIMD kernels can be split into separate modules with\n"
       "  '-split-esimd' option. The option has no effect when there is only\n"
       "  one type of kernels in the input module. Functions unreachable from\n"
-      "  any kernel or SYCL_EXTERNAL function are dropped from the resulting\n"
-      "  module(s)."
+      "  any entry point (kernels and SYCL_EXTERNAL functions) are\n"
+      "  dropped from the resulting module(s).\n"
       "- Module splitter to split a big input module into smaller ones.\n"
       "  Groups kernels using function attribute 'sycl-module-id', i.e.\n"
       "  kernels with the same values of the 'sycl-module-id' attribute will\n"
