[OpenMP][OpenMPIRBuilder] Migrate setPropertyExecutionMode() from Clang to OpenMPIRBuilder.

[raghavendhra]

Summary

Move setPropertyExecutionMode() from Clang to OpenMPIRBuilder and change the references in Clang to use the function from OMPIRBuilder.

[jplehr]

Not an expert here, only some minor comments.

clang/lib/CodeGen/CodeGenModule.h

1015

Return a reference instead of pointer here?

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1506

Rename to sth like isSPMDMode or so? To me Mode reads overly general.

1507

Should this preferably be a reference instead of pointer?

[raghavendhra]

Addressed review comments.

[raghavendhra]

Rebased patch.

[jplehr]

clang/lib/CodeGen/CodeGenModule.h

1016

I believe that this can be simplified to

std::vector<llvm::WeakTrackingVH> &getLLVMCompilerUsed() {
  return LLVMCompilerUsed;
}

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1509

...., bool IsSPMDMode, std::vector<llvm::WeakTrackingVH> &LLVMCompilerUsed);

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4938

...., std::vector<llvm::WeakTrackingVH> &LLVMCompilerUsed) {`

4947

Can this assertion ever be not be met? I'm just curious, given that you create that GVMode yourself.

[raghavendhra]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4947

Adopted this assert from Clang Codegen addCompilerUsedGlobal() which is called inside setPropertyExecutionMode() in CGOpenMPRuntimeGPU.cpp

Actual definition of addCompilerusedGlobal() in CodeGenModule.cpp

void CodeGenModule::addCompilerUsedGlobal(llvm::GlobalValue *GV) {

assert(!GV->isDeclaration() &&
       "Only globals with definition can force usage.");
LLVMCompilerUsed.emplace_back(GV);

}

[raghavendhra]

Addressed JP's review comments and rebased.

[jdoerfert]

Mostly nits. One problem I have is the vector. I do not believe Flang will use the same for their "llvm.used" tracking. I think it makes more sense to provide a callback or just return the global and let the caller handle it.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1508

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4937

4950

No llvm:: here.

[raghavendhra]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4950

Sorry, Can you please elaborate which llvm:: you mentioned about here? Thanks!

[raghavendhra]

Addressed review comments by making setPropertyExecutionMode() to return a Global Value instead of void. Rebased patch.

[raghavendhra]

Remove llvm:: from function setPropertyExecutionMode(). Rebasing the patch.

[jplehr]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4947

Sorry for the confusion I potentially created: I mainly wanted to understand if that case can ever happen.
I'm not really familiar with this part of the compiler.

[jdoerfert]

LG, address the nits below please.

clang/lib/CodeGen/CodeGenModule.h

1017

Unused now.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4941

4949–4956

unrelated. Avoid in this commit, feel free to push a NFC clang-format patch first.

[raghavendhra]

setPropertyExecutionMode function is no longer present / used in clang because of a change in https://reviews.llvm.org/D142569 So abandoning this patch to migrate that function into OMPIRBuilder.

clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

void CGOpenMPRuntimeGPU::emitSPMDKernel(const OMPExecutableDirective &D,
   } Action(*this, EST);
   CodeGen.setAction(Action);
   IsInTTDRegion = true;
   emitTargetOutlinedFunctionHelper(D, ParentName, OutlinedFn, OutlinedFnID,
                                    IsOffloadEntry, CodeGen);
   IsInTTDRegion = false;
 }
 
-// Create a unique global variable to indicate the execution mode of this target
-// region. The execution mode is either 'generic', or 'spmd' depending on the
-// target directive. This variable is picked up by the offload library to setup
-// the device appropriately before kernel launch. If the execution mode is
-// 'generic', the runtime reserves one warp for the master, otherwise, all
-// warps participate in parallel work.
-static void setPropertyExecutionMode(CodeGenModule &CGM, StringRef Name,
-                                     bool Mode) {
-  auto *GVMode = new llvm::GlobalVariable(
-      CGM.getModule(), CGM.Int8Ty, /*isConstant=*/true,
-      llvm::GlobalValue::WeakAnyLinkage,
-      llvm::ConstantInt::get(CGM.Int8Ty, Mode ? OMP_TGT_EXEC_MODE_SPMD
-                                              : OMP_TGT_EXEC_MODE_GENERIC),
-      Twine(Name, "_exec_mode"));
-  GVMode->setVisibility(llvm::GlobalVariable::ProtectedVisibility);
-  CGM.addCompilerUsedGlobal(GVMode);
-}
-
 void CGOpenMPRuntimeGPU::emitTargetOutlinedFunction(
     const OMPExecutableDirective &D, StringRef ParentName,
     llvm::Function *&OutlinedFn, llvm::Constant *&OutlinedFnID,
     bool IsOffloadEntry, const RegionCodeGenTy &CodeGen) {
   if (!IsOffloadEntry) // Nothing to do.
     return;
 
   assert(!ParentName.empty() && "Invalid target region parent name!");
 
   bool Mode = supportsSPMDExecutionMode(CGM.getContext(), D);
   if (Mode)
     emitSPMDKernel(D, ParentName, OutlinedFn, OutlinedFnID, IsOffloadEntry,
                    CodeGen);
   else
     emitNonSPMDKernel(D, ParentName, OutlinedFn, OutlinedFnID, IsOffloadEntry,
                       CodeGen);
 
-  setPropertyExecutionMode(CGM, OutlinedFn->getName(), Mode);
+  llvm::OpenMPIRBuilder &OMPBuilder = getOMPBuilder();
+  llvm::GlobalVariable *GVMode =
+      OMPBuilder.setPropertyExecutionMode(OutlinedFn->getName(), Mode);
+  CGM.addCompilerUsedGlobal(GVMode);
 }
 
 CGOpenMPRuntimeGPU::CGOpenMPRuntimeGPU(CodeGenModule &CGM)
     : CGOpenMPRuntime(CGM) {
   llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsTargetDevice,
                                      isGPU(), hasRequiresUnifiedSharedMemory(),
                                      CGM.getLangOpts().OpenMPOffloadMandatory);
   OMPBuilder.setConfig(Config);

clang/lib/CodeGen/CodeGenModule.h

public:
   llvm::Constant *GetAddrOfGlobalBlock(const BlockExpr *BE, StringRef Name);
 
   /// Returns the address of a block which requires no caputres, or null if
   /// we've yet to emit the block for BE.
   llvm::Constant *getAddrOfGlobalBlockIfEmitted(const BlockExpr *BE) {
     return EmittedGlobalBlocks.lookup(BE);
   }
 
+  std::vector<llvm::WeakTrackingVH> &getLLVMCompilerUsed() {
+    return LLVMCompilerUsed;
+  }
+
   /// Notes that BE's global block is available via Addr. Asserts that BE
   /// isn't already emitted.
   void setAddrOfGlobalBlock(const BlockExpr *BE, llvm::Constant *Addr);
 
   /// Return a pointer to a constant CFString object for the given string.
   ConstantAddress GetAddrOfConstantCFString(const StringLiteral *Literal);
 
   /// Return a constant array for the given string.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

public:
   /// if (Cond) {
   ///   ThenGen();
   /// } else {
   ///   ElseGen();
   /// }
   void emitIfClause(Value *Cond, BodyGenCallbackTy ThenGen,
                     BodyGenCallbackTy ElseGen, InsertPointTy AllocaIP = {});
 
+  /// Create a unique global variable to indicate the execution mode of this
+  /// target region. The execution mode is either 'generic', or 'spmd' depending
+  /// on the target directive. This variable is picked up by the offload library
+  /// to setup the device appropriately before kernel launch. If the execution
+  /// mode is 'generic', the runtime reserves one warp for the master,
+  /// otherwise, all warps participate in parallel work.
+  /// \param Name The symbol name associated with the global.
+  /// \param IsSPMDMode is boolean to indicate if the kernel is an SPMD kernel
+  /// or not.
+  GlobalVariable *setPropertyExecutionMode(StringRef Name, bool IsSPMDMode);
+
   /// Create the global variable holding the offload mappings information.
   GlobalVariable *createOffloadMaptypes(SmallVectorImpl<uint64_t> &Mappings,
                                         std::string VarName);
 
   /// Create the global variable holding the offload names information.
   GlobalVariable *
   createOffloadMapnames(SmallVectorImpl<llvm::Constant *> &Names,
                         std::string VarName);
 
   struct MapperAllocas {
     AllocaInst *ArgsBase = nullptr;

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

void OpenMPIRBuilder::emitIfClause(Value *Cond, BodyGenCallbackTy ThenGen,
   emitBlock(ElseBlock, CurFn);
   ElseGen(AllocaIP, Builder.saveIP());
   // There is no need to emit line number for unconditional branch.
   emitBranch(ContBlock);
   // Emit the continuation block for code after the if.
   emitBlock(ContBlock, CurFn, /*IsFinished=*/true);
 }
 
+GlobalVariable *OpenMPIRBuilder::setPropertyExecutionMode(StringRef Name,
+                                                          bool IsSPMDMode) {
+  auto *GVMode = new GlobalVariable(
+      M, Type::getInt8Ty(M.getContext()), /*isConstant=*/true,
+      GlobalValue::WeakAnyLinkage,
+      ConstantInt::get(llvm::Type::getInt8Ty(M.getContext()),
+                       IsSPMDMode ? OMP_TGT_EXEC_MODE_SPMD
+                                  : OMP_TGT_EXEC_MODE_GENERIC),
+      Twine(Name, "_exec_mode"));
+  GVMode->setVisibility(GlobalVariable::ProtectedVisibility);
+  return GVMode;
+}
+
 bool OpenMPIRBuilder::checkAndEmitFlushAfterAtomic(
-    const LocationDescription &Loc, llvm::AtomicOrdering AO, AtomicKind AK) {
-  assert(!(AO == AtomicOrdering::NotAtomic ||
-           AO == llvm::AtomicOrdering::Unordered) &&
-         "Unexpected Atomic Ordering.");
+    const LocationDescription &Loc, AtomicOrdering AO, AtomicKind AK) {
+  assert(
+      !(AO == AtomicOrdering::NotAtomic || AO == AtomicOrdering::Unordered) &&
 
   bool Flush = false;
-  llvm::AtomicOrdering FlushAO = AtomicOrdering::Monotonic;
+  AtomicOrdering FlushAO = AtomicOrdering::Monotonic;
 
   switch (AK) {
   case Read:
     if (AO == AtomicOrdering::Acquire || AO == AtomicOrdering::AcquireRelease ||
         AO == AtomicOrdering::SequentiallyConsistent) {
       FlushAO = AtomicOrdering::Acquire;
       Flush = true;
     }