Injection of kernel features into the LLVM IR during the OpenMP transform stage

[ksidorov]

Summary

This patch contains:

• the code for retrieving & injecting features of the kernel into the LLVM IR as the global [* x i64] variable,
• the regression test that checks the injected feature vectors for (a) empty kernel and (b) matrix multiplication kernel.

[jdoerfert]

Some minor thing but overall this looks sensible.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

578

We need a command line option to guard this. Doing it always is (for now) not helpful.

1515–1516

Or use LLVM_DEBUG({ ... }); to avoid two consecutive ones.

1521

Early exits are preferred. if (!FunctionInfo) { debug; continue; } will make it easier to read.

[ksidorov]

• Command line option -openmp-inject-kernel-features for turning on the feature injection.
• Refactoring + formatting issues.

[jdoerfert]

squash the commits so that we see the diff not from the last change but the entire change set

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

1499

No else.

[ksidorov]

Squash of diff versions

[jdoerfert]

LG, one nit. Once the nit is resolved I can commit this for you.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

1522

Address the warning, maybe just (void) FeatureVector;

[ksidorov]

Code review comments: (void) + clang-format issues

[JonChesterfield]

Good feature, but using an array for this looks likely to be error prone. Do we need to iterate over it somewhere? Order of fields seems likely to end up repeated in a few places.

I think I'd prefer N distinct scalars with names like multiply_kernel.KernelFeature.MaxLoopDepth. Might make the test cases more legible too.

[jdoerfert]

In D103770#2881332, @JonChesterfield wrote:

Good feature, but using an array for this looks likely to be error prone. Do we need to iterate over it somewhere? Order of fields seems likely to end up repeated in a few places.

I think I'd prefer N distinct scalars with names like multiply_kernel.KernelFeature.MaxLoopDepth. Might make the test cases more legible too.

When we read out the values it is much more reasonable to do so in one go. More features are on the way, reading them out one by one will cost too much time to make it practical.
With the usage and printing in the runtime D109882 this should make more sense.

That said, we should even merge other globals into a single one, incl. exec_mode.

[ksidorov]

Aligned the order of kernel features to the order in the struct definition

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

 //===-- IPO/OpenMPOpt.cpp - Collection of OpenMP specific optimizations ---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
if (IsModulePass) {
       Changed |= deduplicateRuntimeCalls();
       if (EnableParallelRegionMerging) {
         if (mergeParallelRegions()) {
           deduplicateRuntimeCalls();
           Changed = true;
         }
       }
 
       if (InjectKernelFeatures)
         Changed |= injectKernelFeatures();
     }
 
     return Changed;
   }
 
   /// Print initial ICV values for testing.
   /// FIXME: This should be done from the Attributor once it is added.
for (Function *F : SCC) {
           OMPInfoCache.getAnalysisResultForFunction<FunctionPropertiesAnalysis>(
               *F);
       if (!FunctionInfo) {
         // OMPInfoCache has not returned the analysis result -- notify the debug
         // output and proceed to the next kernel
         LLVM_DEBUG(dbgs() << TAG
                           << " No valid function info received for kernel "
                           << F->getName() << "\n");
         continue;
       }
       // OMPInfoCache has returned the analysis result -- pack the feature
       // values into the [* x i64] array and inject them as the constant
-      // global variable with the name '$KernelName.KernelFeatures'
+      // global variable with the name '$KernelName_KernelFeatures'
       IntegerType *FeatureType = Type::getInt64Ty(M.getContext());
       ArrayRef<Constant *> FeatureArrayRef{
           ConstantInt::get(FeatureType, FunctionInfo->BasicBlockCount),
           ConstantInt::get(
               FeatureType,
               FunctionInfo->BlocksReachedFromConditionalInstruction),
+          ConstantInt::get(FeatureType, FunctionInfo->Uses),
           ConstantInt::get(FeatureType,
                            FunctionInfo->DirectCallsToDefinedFunctions),
           ConstantInt::get(FeatureType, FunctionInfo->LoadInstCount),
-          ConstantInt::get(FeatureType, FunctionInfo->MaxLoopDepth),
           ConstantInt::get(FeatureType, FunctionInfo->StoreInstCount),
+          ConstantInt::get(FeatureType, FunctionInfo->MaxLoopDepth),
           ConstantInt::get(FeatureType, FunctionInfo->TopLevelLoopCount),
-          ConstantInt::get(FeatureType, FunctionInfo->Uses),
       };
       ArrayType *FeatureArrayType =
           ArrayType::get(FeatureType, FeatureArrayRef.size());
       Constant *FeatureArray =
           ConstantArray::get(FeatureArrayType, FeatureArrayRef);
       GlobalVariable *FeatureVector = new GlobalVariable(
           M, FeatureArrayType, true, GlobalValue::ExternalLinkage, FeatureArray,
-          F->getName() + ".KernelFeatures");
+          F->getName() + "_KernelFeatures");
       (void)FeatureVector;
       IsChanged = true;
       LLVM_DEBUG({
         FeatureArray->print(dbgs() << TAG << " Function info for kernel "
                                    << F->getName() << " is: ");
         dbgs() << "\n";
       });
     }

llvm/test/Transforms/OpenMP/gpu_kernel_features.ll

 ; RUN: opt -passes=openmp-opt-cgscc -openmp-inject-kernel-features -S < %s | FileCheck %s
 
-; CHECK-DAG: @empty_kernel1.KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 1]
-; CHECK-DAG: @empty_kernel2.KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 1]
-; CHECK-DAG: @matmul_kernel.KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 1, i64 0, i64 0, i64 1, i64 0, i64 1]
-; CHECK-DAG: @multiply_kernel.KernelFeatures = constant [8 x i64] [i64 13, i64 6, i64 0, i64 21, i64 3, i64 11, i64 1, i64 2]
+; CHECK-DAG: @empty_kernel1_KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 1, i64 0, i64 0, i64 0, i64 0, i64 0]
+; CHECK-DAG: @empty_kernel2_KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 1, i64 0, i64 0, i64 0, i64 0, i64 0]
+; CHECK-DAG: @matmul_kernel_KernelFeatures = constant [8 x i64] [i64 1, i64 0, i64 1, i64 1, i64 0, i64 1, i64 0, i64 0]
+; CHECK-DAG: @multiply_kernel_KernelFeatures = constant [8 x i64] [i64 13, i64 6, i64 2, i64 0, i64 21, i64 11, i64 3, i64 1]
 
 define void @empty_kernel1() {
   ret void
 }
 
 define void @empty_kernel2() {
   ret void
 }