[NFC][SYCL] Apply post-commit comments to ITT annotation support (#3691)

Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>

Author: MrSidims

clang/lib/CodeGen/BackendUtil.cpp

@@ -994,7 +994,7 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction Action,
     if (!llvm::Triple(TheModule->getTargetTriple()).isSPIR())
       llvm::report_fatal_error(
           "ITT annotations can only by added to a module with spir target");
-    PerModulePasses.add(createSPIRITTAnnotationsPass());
+    PerModulePasses.add(createSPIRITTAnnotationsLegacyPass());
   }
 
   // Allocate static local memory in SYCL kernel scope for each allocation call.

llvm/include/llvm/LinkAllPasses.h

@@ -206,7 +206,7 @@ namespace {
       (void)llvm::createSYCLLowerESIMDPass();
       (void)llvm::createESIMDLowerLoadStorePass();
       (void)llvm::createESIMDLowerVecArgPass();
-      (void)llvm::createSPIRITTAnnotationsPass();
+      (void)llvm::createSPIRITTAnnotationsLegacyPass();
       (void)llvm::createSYCLLowerWGLocalMemoryLegacyPass();
       std::string buf;
       llvm::raw_string_ostream os(buf);

llvm/include/llvm/Transforms/Instrumentation/SPIRITTAnnotations.h

@@ -22,6 +22,6 @@ class SPIRITTAnnotationsPass : public PassInfoMixin<SPIRITTAnnotationsPass> {
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
 };
 
-ModulePass *createSPIRITTAnnotationsPass();
+ModulePass *createSPIRITTAnnotationsLegacyPass();
 
 } // namespace llvm

llvm/lib/Transforms/Instrumentation/SPIRITTAnnotations.cpp

@@ -132,7 +132,7 @@ INITIALIZE_PASS(SPIRITTAnnotationsLegacyPass, "SPIRITTAnnotations",
                 "Insert ITT annotations in SPIR code", false, false)
 
 // Public interface to the SPIRITTAnnotationsPass.
-ModulePass *llvm::createSPIRITTAnnotationsPass() {
+ModulePass *llvm::createSPIRITTAnnotationsLegacyPass() {
   return new SPIRITTAnnotationsLegacyPass();
 }
 
@@ -151,35 +151,32 @@ Instruction *emitCall(Module &M, Type *RetTy, StringRef FunctionName,
   auto *FT = FunctionType::get(RetTy, ArgTys, false /*isVarArg*/);
   FunctionCallee FC = M.getOrInsertFunction(FunctionName, FT);
   assert(FC.getCallee() && "Instruction creation failed");
-  auto *Call = CallInst::Create(FT, FC.getCallee(), Args, "", InsertBefore);
-  return Call;
+  return CallInst::Create(FT, FC.getCallee(), Args, "", InsertBefore);
 }
 
 // Insert instrumental annotation calls, that has no arguments (for example
 // work items start/finish/resume and barrier annotation.
-bool insertSimpleInstrumentationCall(Module &M, StringRef Name,
+void insertSimpleInstrumentationCall(Module &M, StringRef Name,
                                      Instruction *Position) {
   Type *VoidTy = Type::getVoidTy(M.getContext());
   ArrayRef<Value *> Args;
   Instruction *InstrumentationCall = emitCall(M, VoidTy, Name, Args, Position);
   assert(InstrumentationCall && "Instrumentation call creation failed");
-  return true;
 }
 
 // Insert instrumental annotation calls for SPIR-V atomics.
-bool insertAtomicInstrumentationCall(Module &M, StringRef Name,
-                                     CallInst *AtomicFun,
-                                     Instruction *Position) {
+void insertAtomicInstrumentationCall(Module &M, StringRef Name,
+                                     CallInst *AtomicFun, Instruction *Position,
+                                     StringRef AtomicName) {
   LLVMContext &Ctx = M.getContext();
   Type *VoidTy = Type::getVoidTy(Ctx);
-  Type *Int32Ty = Type::getInt32Ty(Ctx);
+  IntegerType *Int32Ty = IntegerType::get(Ctx, 32);
   // __spirv_Atomic... instructions have following arguments:
   // Pointer, Memory Scope, Memory Semantics and others. To construct Atomic
   // annotation instructions we need Pointer and Memory Semantic arguments
   // taken from the original Atomic instruction.
   Value *Ptr = dyn_cast<Value>(AtomicFun->getArgOperand(0));
-  StringRef AtomicName = AtomicFun->getCalledFunction()->getName();
-  Value *AtomicOp;
+  assert(Ptr && "Failed to get a pointer argument of atomic instruction");
   // Second parameter of Atomic Start/Finish annotation is an Op code of
   // the instruction, encoded into a value of enum, defined like this on user's/
   // profiler's side:
@@ -189,12 +186,11 @@ bool insertAtomicInstrumentationCall(Module &M, StringRef Name,
   //   __itt_mem_store = 1,
   //   __itt_mem_update = 2
   // }
-  if (AtomicName.contains(SPIRV_ATOMIC_LOAD))
-    AtomicOp = ConstantInt::get(Int32Ty, 0);
-  else if (AtomicName.contains(SPIRV_ATOMIC_STORE))
-    AtomicOp = ConstantInt::get(Int32Ty, 1);
-  else
-    AtomicOp = ConstantInt::get(Int32Ty, 2);
+  ConstantInt *AtomicOp =
+      StringSwitch<ConstantInt *>(AtomicName)
+          .StartsWith(SPIRV_ATOMIC_LOAD, ConstantInt::get(Int32Ty, 0))
+          .StartsWith(SPIRV_ATOMIC_STORE, ConstantInt::get(Int32Ty, 1))
+          .Default(ConstantInt::get(Int32Ty, 2));
   // Third parameter of Atomic Start/Finish annotation is an ordering
   // semantic of the instruction, encoded into a value of enum, defined like
   // this on user's/profiler's side:
@@ -209,23 +205,24 @@ bool insertAtomicInstrumentationCall(Module &M, StringRef Name,
   // differencies in values between SYCL mem order and SPIR-V mem order, SYCL RT
   // also applies Memory Semantic mask, like WorkgroupMemory (0x100)), need to
   // align it.
-  uint64_t MemFlag = dyn_cast<ConstantInt>(AtomicFun->getArgOperand(2))
-                         ->getValue()
-                         .getZExtValue();
+  auto *MemFlag = dyn_cast<ConstantInt>(AtomicFun->getArgOperand(2));
+  // TODO: add non-constant memory order processing
+  if (!MemFlag)
+    return;
+  uint64_t IntMemFlag = MemFlag->getValue().getZExtValue();
   uint64_t Order;
-  if (MemFlag & 0x2)
+  if (IntMemFlag & 0x2)
     Order = 1;
-  else if (MemFlag & 0x4)
+  else if (IntMemFlag & 0x4)
     Order = 2;
-  else if (MemFlag & 0x8)
+  else if (IntMemFlag & 0x8)
     Order = 3;
   else
     Order = 0;
   Value *MemOrder = ConstantInt::get(Int32Ty, Order);
   Value *Args[] = {Ptr, AtomicOp, MemOrder};
   Instruction *InstrumentationCall = emitCall(M, VoidTy, Name, Args, Position);
   assert(InstrumentationCall && "Instrumentation call creation failed");
-  return true;
 }
 
 } // namespace
@@ -249,15 +246,14 @@ PreservedAnalyses SPIRITTAnnotationsPass::run(Module &M,
     // At the beggining of a kernel insert work item start annotation
     // instruction.
     if (IsSPIRKernel)
-      IRModified |= insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WI_START,
-                                                    &*inst_begin(F));
+      insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WI_START,
+                                      &*inst_begin(F));
 
     for (BasicBlock &BB : F) {
       // Insert Finish instruction before return instruction
       if (IsSPIRKernel)
         if (ReturnInst *RI = dyn_cast<ReturnInst>(BB.getTerminator()))
-          IRModified |=
-              insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WI_FINISH, RI);
+          insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WI_FINISH, RI);
       for (Instruction &I : BB) {
         CallInst *CI = dyn_cast<CallInst>(&I);
         if (!CI)
@@ -279,16 +275,15 @@ PreservedAnalyses SPIRITTAnnotationsPass::run(Module &M,
                           return CalleeName.startswith(Name);
                         })) {
           Instruction *InstAfterBarrier = CI->getNextNode();
-          IRModified |=
-              insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WG_BARRIER, CI);
-          IRModified |= insertSimpleInstrumentationCall(
-              M, ITT_ANNOTATION_WI_RESUME, InstAfterBarrier);
+          insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WG_BARRIER, CI);
+          insertSimpleInstrumentationCall(M, ITT_ANNOTATION_WI_RESUME,
+                                          InstAfterBarrier);
         } else if (CalleeName.startswith(SPIRV_ATOMIC_INST)) {
           Instruction *InstAfterAtomic = CI->getNextNode();
-          IRModified |= insertAtomicInstrumentationCall(
-              M, ITT_ANNOTATION_ATOMIC_START, CI, CI);
-          IRModified |= insertAtomicInstrumentationCall(
-              M, ITT_ANNOTATION_ATOMIC_FINISH, CI, InstAfterAtomic);
+          insertAtomicInstrumentationCall(M, ITT_ANNOTATION_ATOMIC_START, CI,
+                                          CI, CalleeName);
+          insertAtomicInstrumentationCall(M, ITT_ANNOTATION_ATOMIC_FINISH, CI,
+                                          InstAfterAtomic, CalleeName);
         }
       }
     }