Prefix the name of the calling host function in the name of callee GPU kernel

[singam-sanjay]

Summary

Provide more context to the name of a GPU kernel by prefixing its name with the host function that calls it. E.g. The first kernel called by gemm would be FUNC_gemm_KERNEL_0.

Kernels currently follow the "kernel_#" (# = 0,1,2,3,...) nomenclature. This patch makes it easier to map host caller and device callee, especially when there are many kernels produced by Polly-ACC.

[grosser]

Nice. The summary reads well. Please add a test case and this should be ready to commit!

[singam-sanjay]

Updated exiting test cases to reflect effect of the patch.

[singam-sanjay]

I've corrected the test cases which are affected by the patch. Should I include a separate patch to highlight the effects of just this patch ?

[singam-sanjay]

Ping @grosser

[grosser]

LGTM.

Can you please ask Chris to give you commit access?

[singam-sanjay]

Sure.

[singam-sanjay]

Prefix SCoP name as well to uniquely identify a kernel, since a function can have more than one SCoP.

[singam-sanjay]

@grosser ping.

[singam-sanjay]

1. Clearly mention the source FUNCtion and SCOP and KERNEL number.
2. Changed name of SCOP prefixed to kernel to follow PTX identifier syntax acc. to http://docs.nvidia.com/cuda/parallel-thread-execution/index.html#identifiers (Raised by @philip.pfaffe )

@kbarton This patch doesn't have anything to do with the PowerPC backend. Could you please update your rules to search for "PowerPC" (might be in the comments) or drop files with "PPCGCodeGeneration" ?

[bollu]

@singam-sanjay I've added style comments. Feel free to ignore the const stuff, but please do remove the code duplication. Other than that, LGTM.

I feel very strongly about immutability, but I understand that using const irregularly is worse than not having const at all. However, I'd like to have const in as many low-friction places as possible, simply because it makes reading code nicer :)

lib/CodeGen/PPCGCodeGeneration.cpp

209 ↗

(On Diff #103753)

Have you considered const std::string? Since we know that this is immutable, we can initialise it once in the constructor of GPUNodeBuilder.

242 ↗

(On Diff #103753)

Could this be a static function? static string createScopNameForIR(const Scop &S). I feel that this carries intent better, because it does not use the state of GPUNodeBuilder at all.

Then, the constructor of GPUNodeBuilder can be

GPUNodeBuilder::GPUNodeBuilder(Scop &S, ...) : ScopNameForIR(createScopNameForIR(S)), ... { }

1492 ↗

(On Diff #103753)

Name creation seems to be replicated here and on line 1450, line 1757. Can this be factored out into a common function?

static std::string CreateKernelIdentifier(ppcg_kernel *Kernel, const Scop &S) { 
   return  "FUNC_" + S.getFunction().getName().str() + 
           "_SCOP_" + getScopNameForIR() + "_KERNEL_" + 
            std::to_string(Kernel->id);
}

[singam-sanjay]

lib/CodeGen/PPCGCodeGeneration.cpp

209 ↗

(On Diff #103753)

I'll change it. Do you think ScopNameForIR should be made a member of Scop ? ( I could add this to D34565 )

242 ↗

(On Diff #103753)

Yes, I agree. Instead, can I make this function a part of class Scop ? Since this is only associated with a Scop object.

1492 ↗

(On Diff #103753)

Sure. Do you think this should be a part of GPUNodeBuilder ?

[grosser]

Hi Sanjay,

adding the scop's name is a good idea. Specifically, the scop's name may differ between release and debug builds, as names are only carried over in debug builds. Having the compiler's output depend on the build settings is something we avoid in LLVM. If we want to distigush scops in the same function, but different scops we probably want to number the scops / regions, rather than using their names. I suggest we do this in another patch, to not further hold this patch back.

Sorry for the delay.

[grosser]

I meant, adding the scop's name is _not_ a good idea.

[singam-sanjay]

In D33985#797009, @grosser wrote:

Hi Sanjay,

adding the scop's name is a not good idea. Specifically, the scop's name may differ between release and debug builds, as names are only carried over in debug builds.

The name of the Scop is built from the boundary basic blocks of the region in Scop::getNameStr. How does that depend on the build type ? Are extra basic blocks inserted in debug builds ?

AFAIK, LLVM Debug build lets us stepthrough and debug the LLVM source.

Having the compiler's output depend on the build settings is something we avoid in LLVM.
If we want to distigush scops in the same function, but different scops we probably want to number the scops / regions, rather than using their names. I suggest we do this in another patch, to not further hold this patch back.

Sorry for the delay.

[singam-sanjay]

@grosser ping

[singam-sanjay]

1. Moved building the name into a routine.
2. ScopNameofIR is const now.
3. Rebased to trunk @ 306540.

[grosser]

In D33985#797819, @singam-sanjay wrote:

In D33985#797009, @grosser wrote:

Hi Sanjay,

adding the scop's name is a not good idea. Specifically, the scop's name may differ between release and debug builds, as names are only carried over in debug builds.

The name of the Scop is built from the boundary basic blocks of the region in Scop::getNameStr. How does that depend on the build type ? Are extra basic blocks inserted in debug builds ?

AFAIK, LLVM Debug build lets us stepthrough and debug the LLVM source.

clang (and LLVM) will name basic blocks, but it will not carry name information in release mode. I suggest to remove the SCOP names for now, commit the majority of the patch and open a new discussion for scop names.

Just run '-S -emit-llvm' on a simple program, once using a clang version compiled with BUILD_TYPE=Debug and once with BUILD_TYPE=Release.

Best,
Tobias

Having the compiler's output depend on the build settings is something we avoid in LLVM.
If we want to distigush scops in the same function, but different scops we probably want to number the scops / regions, rather than using their names. I suggest we do this in another patch, to not further hold this patch back.

Sorry for the delay.

[singam-sanjay]

Drop Scop name from ptx_kernel's name.

[grosser]

LGTM, thanks

polly/trunk/lib/CodeGen/PPCGCodeGeneration.cpp

public:
   bool BuildSuccessful = true;
 
   /// The maximal number of loops surrounding a sequential kernel.
   unsigned DeepestSequential = 0;
 
   /// The maximal number of loops surrounding a parallel kernel.
   unsigned DeepestParallel = 0;
 
+  /// Return the name to set for the ptx_kernel.
+  std::string getKernelFuncName(int Kernel_id);
+
 private:
   /// A vector of array base pointers for which a new ScopArrayInfo was created.
   ///
   /// This vector is used to delete the ScopArrayInfo when it is not needed any
   /// more.
   std::vector<Value *> LocalArrays;
 
   /// A map from ScopArrays to their corresponding device allocations.
private:
   /// @param Parameters A pointer to an array that contains itself pointers to
   ///                   the parameter values passed for each kernel argument.
   void createCallLaunchKernel(Value *GPUKernel, Value *GridDimX,
                               Value *GridDimY, Value *BlockDimX,
                               Value *BlockDimY, Value *BlockDimZ,
                               Value *Parameters);
 };
 
+std::string GPUNodeBuilder::getKernelFuncName(int Kernel_id) {
+  return "FUNC_" + S.getFunction().getName().str() + "_KERNEL_" +
+         std::to_string(Kernel_id);
+}
+
 void GPUNodeBuilder::initializeAfterRTH() {
   BasicBlock *NewBB = SplitBlock(Builder.GetInsertBlock(),
                                  &*Builder.GetInsertPoint(), &DT, &LI);
   NewBB->setName("polly.acc.initialize");
   Builder.SetInsertPoint(&NewBB->front());
 
   GPUContext = createCallInitContext();
 
void GPUNodeBuilder::createKernel(__isl_take isl_ast_node *KernelStmt) {
   for (auto &BasePtr : LocalArrays)
     S.invalidateScopArrayInfo(BasePtr, MemoryKind::Array);
   LocalArrays.clear();
 
   std::string ASMString = finalizeKernelFunction();
   Builder.SetInsertPoint(&HostInsertPoint);
   Value *Parameters = createLaunchParameters(Kernel, F, SubtreeValues);
 
-  std::string Name = "kernel_" + std::to_string(Kernel->id);
+  std::string Name = getKernelFuncName(Kernel->id);
   Value *KernelString = Builder.CreateGlobalStringPtr(ASMString, Name);
   Value *NameString = Builder.CreateGlobalStringPtr(Name, Name + "_name");
   Value *GPUKernel = createCallGetKernel(KernelString, NameString);
 
   Value *GridDimX, *GridDimY;
   std::tie(GridDimX, GridDimY) = getGridSizes(Kernel);
 
   createCallLaunchKernel(GPUKernel, GridDimX, GridDimY, BlockDimX, BlockDimY,
static std::string computeNVPTXDataLayout(bool is64Bit) {
 
   return Ret;
 }
 
 Function *
 GPUNodeBuilder::createKernelFunctionDecl(ppcg_kernel *Kernel,
                                          SetVector<Value *> &SubtreeValues) {
   std::vector<Type *> Args;
-  std::string Identifier = "kernel_" + std::to_string(Kernel->id);
+  std::string Identifier = getKernelFuncName(Kernel->id);
 
   for (long i = 0; i < Prog->n_array; i++) {
     if (!ppcg_kernel_requires_array_argument(Kernel, i))
       continue;
 
     if (gpu_array_is_read_only_scalar(&Prog->array[i])) {
       isl_id *Id = isl_space_get_tuple_id(Prog->array[i].space, isl_dim_set);
       const ScopArrayInfo *SAI = ScopArrayInfo::getFromId(Id);
for (int i = 0; i < Kernel->n_var; ++i) {
     KernelIds.push_back(Id);
     IDToSAI[Id] = SAI;
   }
 }
 
 void GPUNodeBuilder::createKernelFunction(
     ppcg_kernel *Kernel, SetVector<Value *> &SubtreeValues,
     SetVector<Function *> &SubtreeFunctions) {
-  std::string Identifier = "kernel_" + std::to_string(Kernel->id);
+  std::string Identifier = getKernelFuncName(Kernel->id);
   GPUModule.reset(new Module(Identifier, Builder.getContext()));
 
   switch (Arch) {
   case GPUArch::NVPTX64:
     if (Runtime == GPURuntime::CUDA)
       GPUModule->setTargetTriple(Triple::normalize("nvptx64-nvidia-cuda"));
     else if (Runtime == GPURuntime::OpenCL)
       GPUModule->setTargetTriple(Triple::normalize("nvptx64-nvidia-nvcl"));

polly/trunk/test/GPGPU/cuda-annotations.ll

 ; RUN: opt %loadPolly -polly-codegen-ppcg -polly-acc-dump-kernel-ir \
 ; RUN: -disable-output < %s | \
 ; RUN: FileCheck -check-prefix=KERNEL %s
 
 ; REQUIRES: pollyacc
 
-; KERNEL: define ptx_kernel void @kernel_0(i8 addrspace(1)* %MemRef_A, i64 %n) #0 {
+; KERNEL: define ptx_kernel void @FUNC_foo_KERNEL_0(i8 addrspace(1)* %MemRef_A, i64 %n) #0 {
 
 ; KERNEL: !nvvm.annotations = !{!0}
 
-; KERNEL: !0 = !{void (i8 addrspace(1)*, i64)* @kernel_0, !"maxntidx", i32 32, !"maxntidy", i32 1, !"maxntidz", i32 1}
+; KERNEL: !0 = !{void (i8 addrspace(1)*, i64)* @FUNC_foo_KERNEL_0, !"maxntidx", i32 32, !"maxntidy", i32 1, !"maxntidz", i32 1}
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 define void @foo(i64* %A, i64 %n) {
 bb:
   br label %bb1
 
 bb1:                                              ; preds = %bb6, %bb

polly/trunk/test/GPGPU/cuda-managed-memory-simple.ll

 ; CHECK-NEXT:  store i32 4, i32* %polly_launch_0_param_size_0
 ; CHECK-NEXT:  %20 = getelementptr [4 x i8*], [4 x i8*]* %polly_launch_0_params, i64 0, i64 2
 ; CHECK-NEXT:  %21 = bitcast i32* %polly_launch_0_param_size_0 to i8*
 ; CHECK-NEXT:  store i8* %21, i8** %20
 ; CHECK-NEXT:  store i32 4, i32* %polly_launch_0_param_size_1
 ; CHECK-NEXT:  %22 = getelementptr [4 x i8*], [4 x i8*]* %polly_launch_0_params, i64 0, i64 3
 ; CHECK-NEXT:  %23 = bitcast i32* %polly_launch_0_param_size_1 to i8*
 ; CHECK-NEXT:  store i8* %23, i8** %22
-; CHECK-NEXT:  %24 = call i8* @polly_getKernel(i8* getelementptr inbounds ([750 x i8], [750 x i8]* @kernel_0, i32 0, i32 0), i8* getelementptr inbounds ([9 x i8], [9 x i8]* @kernel_0_name, i32 0, i32 0))
+; CHECK-NEXT:  %24 = call i8* @polly_getKernel(i8* getelementptr inbounds ([810 x i8], [810 x i8]* @FUNC_copy_KERNEL_0, i32 0, i32 0), i8* getelementptr inbounds ([19 x i8], [19 x i8]* @FUNC_copy_KERNEL_0_name, i32 0, i32 0))
 ; CHECK-NEXT:  call void @polly_launchKernel(i8* %24, i32 2, i32 1, i32 32, i32 1, i32 1, i8* %polly_launch_0_params_i8ptr)
 ; CHECK-NEXT:  call void @polly_freeKernel(i8* %24)
 ; CHECK-NEXT:  call void @polly_synchronizeDevice()
 ; CHECK-NEXT:  call void @polly_freeContext(i8* %13)
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 define void @copy(i32* %R, i32* %A) {

polly/trunk/test/GPGPU/host-control-flow.ll

 ; IR-NEXT:  store i8* [[REGB]], i8** [[REGA]]
 ; IR: call i8* @polly_getKernel
 ; ...
 ; IR: call void @polly_freeKernel
 ; IR-NEXT:   %polly.indvar_next = add nsw i64 %polly.indvar, 1
 ; IR-NEXT:   %polly.loop_cond = icmp sle i64 %polly.indvar_next, 99
 ; IR-NEXT:   br i1 %polly.loop_cond, label %polly.loop_header, label %polly.loop_exit
 
-; KERNEL-IR: define ptx_kernel void @kernel_0(i8 addrspace(1)* %MemRef_A, i64 %c0)
+; KERNEL-IR: define ptx_kernel void @FUNC_foo_KERNEL_0(i8 addrspace(1)* %MemRef_A, i64 %c0)
 ; KERNEL-IR-LABEL: entry:
 ; KERNEL-IR-NEXT:   %0 = call i32 @llvm.nvvm.read.ptx.sreg.ctaid.x()
 ; KERNEL-IR-NEXT:   %b0 = zext i32 %0 to i64
 ; KERNEL-IR-NEXT:   %1 = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 ; KERNEL-IR-NEXT:   %t0 = zext i32 %1 to i64
 ; KERNEL-IR-NEXT:   br label %polly.cond
 
 ; KERNEL-IR-LABEL: polly.cond:                                       ; preds = %entry

polly/trunk/test/GPGPU/invariant-load-hoisting.ll

 ; SCOP-NEXT:          ReadAccess := [Reduction Type: NONE] [Scalar: 0]
 ; SCOP-NEXT:              [n, tmp12] -> { Stmt_for_body6[i0, i1, i2] -> MemRef_invariant[0] };
 ; SCOP-NEXT:          Execution Context: [n, tmp12] -> {  : n > 0 }
 ; SCOP-NEXT:  }
 ;
 ; HOST-IR:      call void @polly_launchKernel(i8* %215, i32 %221, i32 1, i32 32, i32 1, i32 1, i8* %polly_launch_0_params_i8ptr)
 ; HOST-IR-NEXT: call void @polly_freeKernel(i8* %215)
 ;
-; KERNEL-IR: define ptx_kernel void @kernel_0(i8 addrspace(1)* %MemRef_B, i8 addrspace(1)* %MemRef_A, i32 %n, i32 %tmp12) #0 {
+; KERNEL-IR: define ptx_kernel void @FUNC_f_KERNEL_0(i8 addrspace(1)* %MemRef_B, i8 addrspace(1)* %MemRef_A, i32 %n, i32 %tmp12) #0 {
 ;
 ; Check that we generate correct GPU code in case of invariant load hoisting.
 ;
 ;
 ;    static const int N = 3000;
 ;
 ;    void f(int A[N][N], int *invariant, int B[N][N], int n) {
 ;      for (int i = 0; i < n; i++) {

polly/trunk/test/GPGPU/kernel-params-only-some-arrays.ll

 ;    void kernel_params_only_some_arrays(float A[], float B[]) {
 ;      for (long i = 0; i < 32; i++)
 ;        A[i] += 42;
 ;
 ;      for (long i = 0; i < 32; i++)
 ;        B[i] += 42;
 ;    }
 
-; KERNEL: ; ModuleID = 'kernel_0'
-; KERNEL-NEXT: source_filename = "kernel_0"
+; KERNEL: ; ModuleID = 'FUNC_kernel_params_only_some_arrays_KERNEL_0'
+; KERNEL-NEXT: source_filename = "FUNC_kernel_params_only_some_arrays_KERNEL_0"
 ; KERNEL-NEXT: target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v16:16:16-v32:32:32-v64:64:64-v128:128:128-n16:32:64"
 ; KERNEL-NEXT: target triple = "nvptx64-nvidia-cuda"
 
-; KERNEL: define ptx_kernel void @kernel_0(i8 addrspace(1)* %MemRef_A)
+; KERNEL: define ptx_kernel void @FUNC_kernel_params_only_some_arrays_KERNEL_0(i8 addrspace(1)* %MemRef_A)
 ; KERNEL-NEXT:   entry:
 ; KERNEL-NEXT:     %0 = call i32 @llvm.nvvm.read.ptx.sreg.ctaid.x()
 ; KERNEL-NEXT:     %b0 = zext i32 %0 to i64
 ; KERNEL-NEXT:     %1 = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 ; KERNEL-NEXT:     %t0 = zext i32 %1 to i64
 
 ; KERNEL:     ret void
 ; KERNEL-NEXT: }
 
-; KERNEL: ; ModuleID = 'kernel_1'
-; KERNEL-NEXT: source_filename = "kernel_1"
+; KERNEL: ; ModuleID = 'FUNC_kernel_params_only_some_arrays_KERNEL_1'
+; KERNEL-NEXT: source_filename = "FUNC_kernel_params_only_some_arrays_KERNEL_1"
 ; KERNEL-NEXT: target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v16:16:16-v32:32:32-v64:64:64-v128:128:128-n16:32:64"
 ; KERNEL-NEXT: target triple = "nvptx64-nvidia-cuda"
 
-; KERNEL: define ptx_kernel void @kernel_1(i8 addrspace(1)* %MemRef_B)
+; KERNEL: define ptx_kernel void @FUNC_kernel_params_only_some_arrays_KERNEL_1(i8 addrspace(1)* %MemRef_B)
 ; KERNEL-NEXT:   entry:
 ; KERNEL-NEXT:     %0 = call i32 @llvm.nvvm.read.ptx.sreg.ctaid.x()
 ; KERNEL-NEXT:     %b0 = zext i32 %0 to i64
 ; KERNEL-NEXT:     %1 = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 ; KERNEL-NEXT:     %t0 = zext i32 %1 to i64
 
 ; KERNEL:     ret void
 ; KERNEL-NEXT: }

polly/trunk/test/GPGPU/kernel-params-scop-parameter.ll

 ; RUN: opt %loadPolly -polly-codegen-ppcg -polly-acc-dump-kernel-ir \
 ; RUN: -disable-output < %s | \
 ; RUN: FileCheck -check-prefix=KERNEL-IR %s
 
 ; REQUIRES: pollyacc
 
 ;    void kernel_params_scop_parameter(float A[], long n) {
 ;      for (long i = 0; i < n; i++)
 ;        A[i] += 42;
 ;    }
 
-; KERNEL-IR: define ptx_kernel void @kernel_0(i8 addrspace(1)* %MemRef_A, i64 %n)
+; KERNEL-IR: define ptx_kernel void @FUNC_kernel_params_scop_parameter_KERNEL_0(i8 addrspace(1)* %MemRef_A, i64 %n)
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 define void @kernel_params_scop_parameter(float* %A, i64 %n) {
 bb:
   br label %bb1
 
 bb1:                                              ; preds = %bb6, %bb