[llvm] 969d787 - [OpenMP][OMPIRBuilder] Add a configuration class that captures flags that affect codegen

Jan Sjodin via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 06:28:00 PST 2022


Author: Jan Sjodin
Date: 2022-11-22T09:25:04-05:00
New Revision: 969d787a470a801ad23be1fd53bcc166f75454a5

URL: https://github.com/llvm/llvm-project/commit/969d787a470a801ad23be1fd53bcc166f75454a5
DIFF: https://github.com/llvm/llvm-project/commit/969d787a470a801ad23be1fd53bcc166f75454a5.diff

LOG: [OpenMP][OMPIRBuilder] Add a configuration class that captures flags that affect codegen

This patch introudces the OpenMPIRBuilderConfig class which contains various
flags that are needed to lower OMP constructs to LLVM-IR. The purpose is to
keep the flags in one place so they do not have to be passed in every time.
The flags can be set optionally since some uses cases don't rely on functions
that depend on these flags.

Reviewed By: jdoerfert, tschuett

Differential Revision: https://reviews.llvm.org/D138220

Added: 
    

Modified: 
    clang/lib/CodeGen/CGOpenMPRuntime.cpp
    clang/lib/CodeGen/CGOpenMPRuntime.h
    clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
    llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
    llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
    llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 35eda4dca3210..0552fb31bff97 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1062,9 +1062,13 @@ CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM, StringRef FirstSeparator,
     : CGM(CGM), FirstSeparator(FirstSeparator), Separator(Separator),
       OMPBuilder(CGM.getModule()), OffloadEntriesInfoManager() {
   KmpCriticalNameTy = llvm::ArrayType::get(CGM.Int32Ty, /*NumElements*/ 8);
+  llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsDevice, false,
+                                     hasRequiresUnifiedSharedMemory());
 
   // Initialize Types used in OpenMPIRBuilder from OMPKinds.def
   OMPBuilder.initialize();
+  OMPBuilder.setConfig(Config);
+  OffloadEntriesInfoManager.setConfig(Config);
   loadOffloadInfoMetadata();
 }
 
@@ -1910,8 +1914,7 @@ bool CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
     CtorEntryInfo.ParentName = Twine(Buffer, "_ctor").toStringRef(Out);
     OffloadEntriesInfoManager.registerTargetRegionEntryInfo(
         CtorEntryInfo, Ctor, ID,
-        llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryCtor,
-        CGM.getLangOpts().OpenMPIsDevice);
+        llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryCtor);
   }
   if (VD->getType().isDestructedType() != QualType::DK_none) {
     llvm::Constant *Dtor;
@@ -1960,8 +1963,7 @@ bool CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
     DtorEntryInfo.ParentName = Twine(Buffer, "_dtor").toStringRef(Out);
     OffloadEntriesInfoManager.registerTargetRegionEntryInfo(
         DtorEntryInfo, Dtor, ID,
-        llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryDtor,
-        CGM.getLangOpts().OpenMPIsDevice);
+        llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryDtor);
   }
   return CGM.getLangOpts().OpenMPIsDevice;
 }
@@ -2980,10 +2982,8 @@ void CGOpenMPRuntime::createOffloadEntriesAndInfoMetadata() {
     }
   };
 
-  OMPBuilder.createOffloadEntriesAndInfoMetadata(
-      OffloadEntriesInfoManager, isTargetCodegen(),
-      CGM.getLangOpts().OpenMPIsDevice,
-      CGM.getOpenMPRuntime().hasRequiresUnifiedSharedMemory(), ErrorReportFn);
+  OMPBuilder.createOffloadEntriesAndInfoMetadata(OffloadEntriesInfoManager,
+                                                 ErrorReportFn);
 }
 
 /// Loads all the offload entries information from the host IR
@@ -6153,8 +6153,7 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
   // Register the information for the entry associated with this target region.
   OffloadEntriesInfoManager.registerTargetRegionEntryInfo(
       EntryInfo, TargetRegionEntryAddr, OutlinedFnID,
-      llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryTargetRegion,
-      CGM.getLangOpts().OpenMPIsDevice);
+      llvm::OffloadEntriesInfoManager::OMPTargetRegionEntryTargetRegion);
 
   // Add NumTeams and ThreadLimit attributes to the outlined GPU function
   int32_t DefaultValTeams = -1;
@@ -10419,7 +10418,7 @@ void CGOpenMPRuntime::registerTargetGlobalVariable(const VarDecl *VD,
   }
 
   OffloadEntriesInfoManager.registerDeviceGlobalVarEntryInfo(
-      VarName, Addr, VarSize, Flags, Linkage, CGM.getLangOpts().OpenMPIsDevice);
+      VarName, Addr, VarSize, Flags, Linkage);
 }
 
 bool CGOpenMPRuntime::emitTargetGlobal(GlobalDecl GD) {
@@ -10461,6 +10460,7 @@ void CGOpenMPRuntime::processRequiresDirective(const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
     if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
       HasRequiresUnifiedSharedMemory = true;
+      OMPBuilder.Config.setHasRequiresUnifiedSharedMemory(true);
     } else if (const auto *AC =
                    dyn_cast<OMPAtomicDefaultMemOrderClause>(Clause)) {
       switch (AC->getAtomicDefaultMemOrderKind()) {

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h b/clang/lib/CodeGen/CGOpenMPRuntime.h
index 7ba9649f5277a..b70708a46eeb3 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -413,8 +413,7 @@ class CGOpenMPRuntime {
   ///
   llvm::Value *getCriticalRegionLock(StringRef CriticalName);
 
-private:
-
+protected:
   /// Map for SourceLocation and OpenMP runtime library debug locations.
   typedef llvm::DenseMap<SourceLocation, llvm::Value *> OpenMPDebugLocMapTy;
   OpenMPDebugLocMapTy OpenMPDebugLocMap;

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index 9f2f60d892b02..4bfc462efaeaa 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -890,6 +890,11 @@ unsigned CGOpenMPRuntimeGPU::getDefaultLocationReserved2Flags() const {
 
 CGOpenMPRuntimeGPU::CGOpenMPRuntimeGPU(CodeGenModule &CGM)
     : CGOpenMPRuntime(CGM, "_", "$") {
+  llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsDevice, true,
+                                     hasRequiresUnifiedSharedMemory());
+  OMPBuilder.setConfig(Config);
+  OffloadEntriesInfoManager.setConfig(Config);
+
   if (!CGM.getLangOpts().OpenMPIsDevice)
     llvm_unreachable("OpenMP can only handle device code.");
 

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 3bb37ad776638..fe9c95c4f6c55 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -73,13 +73,62 @@ BasicBlock *splitBB(IRBuilder<> &Builder, bool CreateBranch, llvm::Twine Name);
 BasicBlock *splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch,
                               llvm::Twine Suffix = ".split");
 
+/// Captures attributes that affect generating LLVM-IR using the
+/// OpenMPIRBuilder and related classes. Note that not all attributes are
+/// required for all classes or functions. In some use cases the configuration
+/// is not necessary at all, because because the only functions that are called
+/// are ones that are not dependent on the configuration.
+class OpenMPIRBuilderConfig {
+public:
+  /// Flag for specifying if the compilation is done for embedded device code
+  /// or host code.
+  Optional<bool> IsEmbedded;
+
+  /// Flag for specifying if the compilation is done for an offloading target,
+  /// like GPU.
+  Optional<bool> IsTargetCodegen;
+
+  /// Flag for specifying weather a requires unified_shared_memory
+  /// directive is present or not.
+  Optional<bool> HasRequiresUnifiedSharedMemory;
+
+  OpenMPIRBuilderConfig() {}
+  OpenMPIRBuilderConfig(bool IsEmbedded, bool IsTargetCodegen,
+                        bool HasRequiresUnifiedSharedMemory)
+      : IsEmbedded(IsEmbedded), IsTargetCodegen(IsTargetCodegen),
+        HasRequiresUnifiedSharedMemory(HasRequiresUnifiedSharedMemory) {}
+
+  // Convenience getter functions that assert if the value is not present.
+  bool isEmbedded() {
+    assert(IsEmbedded.has_value() && "IsEmbedded is not set");
+    return IsEmbedded.value();
+  }
+
+  bool isTargetCodegen() {
+    assert(IsTargetCodegen.has_value() && "IsTargetCodegen is not set");
+    return IsTargetCodegen.value();
+  }
+
+  bool hasRequiresUnifiedSharedMemory() {
+    assert(HasRequiresUnifiedSharedMemory.has_value() &&
+           "HasUnifiedSharedMemory is not set");
+    return HasRequiresUnifiedSharedMemory.value();
+  }
+
+  void setIsEmbedded(bool Value) { IsEmbedded = Value; }
+  void setIsTargetCodegen(bool Value) { IsTargetCodegen = Value; }
+  void setHasRequiresUnifiedSharedMemory(bool Value) {
+    HasRequiresUnifiedSharedMemory = Value;
+  }
+};
+
 /// An interface to create LLVM-IR for OpenMP directives.
 ///
 /// Each OpenMP directive has a corresponding public generator method.
 class OpenMPIRBuilder {
 public:
   /// Create a new OpenMPIRBuilder operating on the given module \p M. This will
-  /// not have an effect on \p M (see initialize).
+  /// not have an effect on \p M (see initialize)
   OpenMPIRBuilder(Module &M) : M(M), Builder(M.getContext()) {}
   ~OpenMPIRBuilder();
 
@@ -88,6 +137,8 @@ class OpenMPIRBuilder {
   /// before any other method and only once!
   void initialize();
 
+  void setConfig(OpenMPIRBuilderConfig C) { Config = C; }
+
   /// Finalize the underlying module, e.g., by outlining regions.
   /// \param Fn                    The function to be finalized. If not used,
   ///                              all functions are finalized.
@@ -942,6 +993,9 @@ class OpenMPIRBuilder {
   /// \param Ident The ident (ident_t*) describing the query origin.
   Value *getOrCreateThreadID(Value *Ident);
 
+  /// The OpenMPIRBuilder Configuration
+  OpenMPIRBuilderConfig Config;
+
   /// The underlying LLVM-IR module
   Module &M;
 
@@ -1094,11 +1148,10 @@ class OpenMPIRBuilder {
                                     bool EmitDebug = false,
                                     bool ForEndCall = false);
 
-  /// Creates offloading entry for the provided entry ID \a ID,
-  /// address \a Addr, size \a Size, and flags \a Flags.
-  void createOffloadEntry(bool IsTargetCodegen, Constant *ID, Constant *Addr,
-                          uint64_t Size, int32_t Flags,
-                          GlobalValue::LinkageTypes);
+  /// Creates offloading entry for the provided entry ID \a ID, address \a
+  /// Addr, size \a Size, and flags \a Flags.
+  void createOffloadEntry(Constant *ID, Constant *Addr, uint64_t Size,
+                          int32_t Flags, GlobalValue::LinkageTypes);
 
   /// The kind of errors that can occur when emitting the offload entries and
   /// metadata.
@@ -1121,8 +1174,6 @@ class OpenMPIRBuilder {
   // We only generate metadata for function that contain target regions.
   void createOffloadEntriesAndInfoMetadata(
       OffloadEntriesInfoManager &OffloadEntriesInfoManager,
-      bool IsTargetCodegen, bool IsEmbedded,
-      bool HasRequiresUnifiedSharedMemory,
       EmitMetadataErrorReportFunctionTy &ErrorReportFunction);
 
 public:
@@ -1759,9 +1810,12 @@ struct TargetRegionEntryInfo {
 /// Class that manages information about offload code regions and data
 class OffloadEntriesInfoManager {
   /// Number of entries registered so far.
+  OpenMPIRBuilderConfig Config;
   unsigned OffloadingEntriesNum = 0;
 
 public:
+  void setConfig(OpenMPIRBuilderConfig C) { Config = C; }
+
   /// Base class of the entries info.
   class OffloadEntryInfo {
   public:
@@ -1813,7 +1867,8 @@ class OffloadEntriesInfoManager {
   bool empty() const;
   /// Return number of entries defined so far.
   unsigned size() const { return OffloadingEntriesNum; }
-  explicit OffloadEntriesInfoManager() {}
+
+  OffloadEntriesInfoManager() : Config() {}
 
   //
   // Target region entries related.
@@ -1862,8 +1917,7 @@ class OffloadEntriesInfoManager {
   /// Register target region entry.
   void registerTargetRegionEntryInfo(TargetRegionEntryInfo EntryInfo,
                                      Constant *Addr, Constant *ID,
-                                     OMPTargetRegionEntryKind Flags,
-                                     bool IsDevice);
+                                     OMPTargetRegionEntryKind Flags);
   /// Return true if a target region entry with the provided information
   /// exists.
   bool hasTargetRegionEntryInfo(TargetRegionEntryInfo EntryInfo,
@@ -1932,8 +1986,7 @@ class OffloadEntriesInfoManager {
   void registerDeviceGlobalVarEntryInfo(StringRef VarName, Constant *Addr,
                                         int64_t VarSize,
                                         OMPTargetGlobalVarEntryKind Flags,
-                                        GlobalValue::LinkageTypes Linkage,
-                                        bool IsDevice);
+                                        GlobalValue::LinkageTypes Linkage);
   /// Checks if the variable with the given name has been registered already.
   bool hasDeviceGlobalVarEntryInfo(StringRef VarName) const {
     return OffloadEntriesDeviceGlobalVar.count(VarName) > 0;

diff  --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 1e381783c6617..9313721c71b4a 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4683,11 +4683,10 @@ void OpenMPIRBuilder::OutlineInfo::collectBlocks(
   }
 }
 
-void OpenMPIRBuilder::createOffloadEntry(bool IsTargetCodegen, Constant *ID,
-                                         Constant *Addr, uint64_t Size,
-                                         int32_t Flags,
+void OpenMPIRBuilder::createOffloadEntry(Constant *ID, Constant *Addr,
+                                         uint64_t Size, int32_t Flags,
                                          GlobalValue::LinkageTypes) {
-  if (!IsTargetCodegen) {
+  if (!Config.isTargetCodegen()) {
     emitOffloadingEntry(ID, Addr->getName(), Size, Flags);
     return;
   }
@@ -4715,8 +4714,7 @@ void OpenMPIRBuilder::createOffloadEntry(bool IsTargetCodegen, Constant *ID,
 
 // We only generate metadata for function that contain target regions.
 void OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata(
-    OffloadEntriesInfoManager &OffloadEntriesInfoManager, bool IsTargetCodegen,
-    bool IsEmbedded, bool HasRequiresUnifiedSharedMemory,
+    OffloadEntriesInfoManager &OffloadEntriesInfoManager,
     EmitMetadataErrorReportFunctionTy &ErrorFn) {
 
   // If there are no entries, we don't need to do anything.
@@ -4809,7 +4807,7 @@ void OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata(
         ErrorFn(EMIT_MD_TARGET_REGION_ERROR, EntryInfo);
         continue;
       }
-      createOffloadEntry(IsTargetCodegen, CE->getID(), CE->getAddress(),
+      createOffloadEntry(CE->getID(), CE->getAddress(),
                          /*Size=*/0, CE->getFlags(),
                          GlobalValue::WeakAnyLinkage);
     } else if (const auto *CE = dyn_cast<
@@ -4820,7 +4818,7 @@ void OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata(
               CE->getFlags());
       switch (Flags) {
       case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo: {
-        if (IsEmbedded && HasRequiresUnifiedSharedMemory)
+        if (Config.isEmbedded() && Config.hasRequiresUnifiedSharedMemory())
           continue;
         if (!CE->getAddress()) {
           ErrorFn(EMIT_MD_DECLARE_TARGET_ERROR, E.second);
@@ -4832,10 +4830,10 @@ void OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata(
         break;
       }
       case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink:
-        assert(((IsEmbedded && !CE->getAddress()) ||
-                (!IsEmbedded && CE->getAddress())) &&
+        assert(((Config.isEmbedded() && !CE->getAddress()) ||
+                (!Config.isEmbedded() && CE->getAddress())) &&
                "Declaret target link address is set.");
-        if (IsEmbedded)
+        if (Config.isEmbedded())
           continue;
         if (!CE->getAddress()) {
           ErrorFn(EMIT_MD_GLOBAL_VAR_LINK_ERROR, TargetRegionEntryInfo());
@@ -4851,8 +4849,8 @@ void OpenMPIRBuilder::createOffloadEntriesAndInfoMetadata(
         if (GV->hasLocalLinkage() || GV->hasHiddenVisibility())
           continue;
 
-      createOffloadEntry(IsTargetCodegen, CE->getAddress(), CE->getAddress(),
-                         CE->getVarSize(), Flags, CE->getLinkage());
+      createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize(),
+                         Flags, CE->getLinkage());
 
     } else {
       llvm_unreachable("Unsupported entry kind.");
@@ -4958,7 +4956,7 @@ void OffloadEntriesInfoManager::initializeTargetRegionEntryInfo(
 
 void OffloadEntriesInfoManager::registerTargetRegionEntryInfo(
     TargetRegionEntryInfo EntryInfo, Constant *Addr, Constant *ID,
-    OMPTargetRegionEntryKind Flags, bool IsDevice) {
+    OMPTargetRegionEntryKind Flags) {
   assert(EntryInfo.Count == 0 && "expected default EntryInfo");
 
   // Update the EntryInfo with the next available count for this location.
@@ -4966,7 +4964,7 @@ void OffloadEntriesInfoManager::registerTargetRegionEntryInfo(
 
   // If we are emitting code for a target, the entry is already initialized,
   // only has to be registered.
-  if (IsDevice) {
+  if (Config.isEmbedded()) {
     // This could happen if the device compilation is invoked standalone.
     if (!hasTargetRegionEntryInfo(EntryInfo)) {
       return;
@@ -5020,9 +5018,8 @@ void OffloadEntriesInfoManager::initializeDeviceGlobalVarEntryInfo(
 
 void OffloadEntriesInfoManager::registerDeviceGlobalVarEntryInfo(
     StringRef VarName, Constant *Addr, int64_t VarSize,
-    OMPTargetGlobalVarEntryKind Flags, GlobalValue::LinkageTypes Linkage,
-    bool IsDevice) {
-  if (IsDevice) {
+    OMPTargetGlobalVarEntryKind Flags, GlobalValue::LinkageTypes Linkage) {
+  if (Config.isEmbedded()) {
     // This could happen if the device compilation is invoked standalone.
     if (!hasDeviceGlobalVarEntryInfo(VarName))
       return;

diff  --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 36c70ee1f54bd..d991b8c931aca 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -5504,17 +5504,18 @@ TEST_F(OpenMPIRBuilderTest, EmitOffloadingArraysArguments) {
 
 TEST_F(OpenMPIRBuilderTest, OffloadEntriesInfoManager) {
   OffloadEntriesInfoManager InfoManager;
+  InfoManager.setConfig(OpenMPIRBuilderConfig(true, false, false));
   TargetRegionEntryInfo EntryInfo("parent", 1, 2, 4, 0);
   InfoManager.initializeTargetRegionEntryInfo(EntryInfo, 0);
-  EXPECT_TRUE(InfoManager.hasTargetRegionEntryInfo(EntryInfo, true));
+  EXPECT_TRUE(InfoManager.hasTargetRegionEntryInfo(EntryInfo));
   InfoManager.initializeDeviceGlobalVarEntryInfo(
       "gvar", OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo, 0);
   InfoManager.registerTargetRegionEntryInfo(
       EntryInfo, nullptr, nullptr,
-      OffloadEntriesInfoManager::OMPTargetRegionEntryTargetRegion, true);
+      OffloadEntriesInfoManager::OMPTargetRegionEntryTargetRegion);
   InfoManager.registerDeviceGlobalVarEntryInfo(
       "gvar", 0x0, 8, OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo,
-      GlobalValue::WeakAnyLinkage, true);
+      GlobalValue::WeakAnyLinkage);
   EXPECT_TRUE(InfoManager.hasDeviceGlobalVarEntryInfo("gvar"));
 }
 } // namespace


        


More information about the llvm-commits mailing list