[clang] 3090232 - [Clang][OpenMP] Rework recently moved getTargetEntryUniqueInfo to fix incorrect error breaking sanitizer

[Andrew Gozillon via cfe-commits]

Author: Andrew Gozillon
Date: 2023-06-07T14:30:25-05:00
New Revision: 309023263dba3b02bc885101faa47d110f662f99

URL: https://github.com/llvm/llvm-project/commit/309023263dba3b02bc885101faa47d110f662f99
DIFF: https://github.com/llvm/llvm-project/commit/309023263dba3b02bc885101faa47d110f662f99.diff

LOG: [Clang][OpenMP] Rework recently moved getTargetEntryUniqueInfo to fix incorrect error breaking sanitizer

This move was done by https://reviews.llvm.org/rGcda46cc4f921f6b288c57a68b901ec2f57134606 and may be the issue causing the sanitizer to fail. But in either case, it is an invalid assert that required some changes to function correctly.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index c7d62fc075fbb..a5a5ed1c98ae0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1646,6 +1646,25 @@ convertCaptureClause(const VarDecl *VD) {
   }
 }
 
+static llvm::TargetRegionEntryInfo getEntryInfoFromPresumedLoc(
+    CodeGenModule &CGM, llvm::OpenMPIRBuilder &OMPBuilder,
+    SourceLocation BeginLoc, llvm::StringRef ParentName = "") {
+
+  auto FileInfoCallBack = [&]() {
+    SourceManager &SM = CGM.getContext().getSourceManager();
+    PresumedLoc PLoc = SM.getPresumedLoc(BeginLoc);
+
+    llvm::sys::fs::UniqueID ID;
+    if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
+      PLoc = SM.getPresumedLoc(BeginLoc, /*UseLineDirectives=*/false);
+    }
+
+    return std::pair<std::string, uint64_t>(PLoc.getFilename(), PLoc.getLine());
+  };
+
+  return OMPBuilder.getTargetEntryUniqueInfo(FileInfoCallBack, ParentName);
+}
+
 Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
   auto AddrOfGlobal = [&VD, this]() { return CGM.GetAddrOfGlobal(VD); };
 
@@ -1654,11 +1673,6 @@ Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
   };
 
   std::vector<llvm::GlobalVariable *> GeneratedRefs;
-  auto PLoc = CGM.getContext().getSourceManager().getPresumedLoc(
-      VD->getCanonicalDecl()->getBeginLoc());
-  if (PLoc.isInvalid())
-    PLoc = CGM.getContext().getSourceManager().getPresumedLoc(
-        VD->getCanonicalDecl()->getBeginLoc(), /*UseLineDirectives=*/false);
 
   llvm::Type *LlvmPtrTy = CGM.getTypes().ConvertTypeForMem(
       CGM.getContext().getPointerType(VD->getType()));
@@ -1666,7 +1680,8 @@ Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
       convertCaptureClause(VD), convertDeviceClause(VD),
       VD->hasDefinition(CGM.getContext()) == VarDecl::DeclarationOnly,
       VD->isExternallyVisible(),
-      OMPBuilder.getTargetEntryUniqueInfo(PLoc.getFilename(), PLoc.getLine()),
+      getEntryInfoFromPresumedLoc(CGM, OMPBuilder,
+                                  VD->getCanonicalDecl()->getBeginLoc()),
       CGM.getMangledName(VD), GeneratedRefs, CGM.getLangOpts().OpenMPSimd,
       CGM.getLangOpts().OMPTargetTriples, LlvmPtrTy, AddrOfGlobal,
       LinkageForVariable);
@@ -1849,19 +1864,6 @@ llvm::Function *CGOpenMPRuntime::emitThreadPrivateVarDefinition(
   return nullptr;
 }
 
-static llvm::TargetRegionEntryInfo getEntryInfoFromPresumedLoc(
-    CodeGenModule &CGM, llvm::OpenMPIRBuilder &OMPBuilder,
-    SourceLocation BeginLoc, llvm::StringRef ParentName) {
-
-  auto PLoc = CGM.getContext().getSourceManager().getPresumedLoc(BeginLoc);
-  if (PLoc.isInvalid())
-    PLoc = CGM.getContext().getSourceManager().getPresumedLoc(
-        BeginLoc, /*UseLineDirectives=*/false);
-
-  return OMPBuilder.getTargetEntryUniqueInfo(PLoc.getFilename(), PLoc.getLine(),
-                                             ParentName);
-}
-
 bool CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
                                                      llvm::GlobalVariable *Addr,
                                                      bool PerformInit) {
@@ -10349,16 +10351,12 @@ void CGOpenMPRuntime::registerTargetGlobalVariable(const VarDecl *VD,
   };
 
   std::vector<llvm::GlobalVariable *> GeneratedRefs;
-  auto PLoc = CGM.getContext().getSourceManager().getPresumedLoc(
-      VD->getCanonicalDecl()->getBeginLoc());
-  if (PLoc.isInvalid())
-    PLoc = CGM.getContext().getSourceManager().getPresumedLoc(
-        VD->getCanonicalDecl()->getBeginLoc(), /*UseLineDirectives=*/false);
   OMPBuilder.registerTargetGlobalVariable(
       convertCaptureClause(VD), convertDeviceClause(VD),
       VD->hasDefinition(CGM.getContext()) == VarDecl::DeclarationOnly,
       VD->isExternallyVisible(),
-      OMPBuilder.getTargetEntryUniqueInfo(PLoc.getFilename(), PLoc.getLine()),
+      getEntryInfoFromPresumedLoc(CGM, OMPBuilder,
+                                  VD->getCanonicalDecl()->getBeginLoc()),
       CGM.getMangledName(VD), GeneratedRefs, CGM.getLangOpts().OpenMPSimd,
       CGM.getLangOpts().OMPTargetTriples, AddrOfGlobal, LinkageForVariable,
       CGM.getTypes().ConvertTypeForMem(

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 0a55229470238..4ae9192b0b60d 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1155,15 +1155,18 @@ class OpenMPIRBuilder {
                                 InsertPointTy AllocaIP,
                                 BodyGenCallbackTy BodyGenCB);
 
+
+  using FileIdentifierInfoCallbackTy = std::function<std::tuple<std::string, uint64_t>()>;
+
   /// Creates a unique info for a target entry when provided a filename and
   /// line number from.
   ///
-  /// \param FileName The name of the file the target entry resides in
-  /// \param Line The line number where the target entry resides
+  /// \param CallBack A callback function which should return filename the entry
+  /// resides in as well as the line number for the target entry
   /// \param ParentName The name of the parent the target entry resides in, if
   /// any.
   static TargetRegionEntryInfo
-  getTargetEntryUniqueInfo(StringRef FileName, uint64_t Line,
+  getTargetEntryUniqueInfo(FileIdentifierInfoCallbackTy CallBack,
                            StringRef ParentName = "");
 
   /// Functions used to generate reductions. Such functions take two Values

diff  --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index b2c556bdae451..2e43e6bb250f6 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -5195,14 +5195,17 @@ void OffloadEntriesInfoManager::getTargetRegionEntryFnName(
 }
 
 TargetRegionEntryInfo
-OpenMPIRBuilder::getTargetEntryUniqueInfo(StringRef FileName, uint64_t Line,
+OpenMPIRBuilder::getTargetEntryUniqueInfo(FileIdentifierInfoCallbackTy CallBack,
                                           StringRef ParentName) {
   sys::fs::UniqueID ID;
-  if (auto EC = sys::fs::getUniqueID(FileName, ID)) {
-    assert(EC &&
-           "Unable to get unique ID for file, during getTargetEntryUniqueInfo");
+  auto FileIDInfo = CallBack();
+  if (auto EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID)) {
+    llvm_unreachable(
+        "Unable to get unique ID for file, during getTargetEntryUniqueInfo");
   }
-  return TargetRegionEntryInfo(ParentName, ID.getDevice(), ID.getFile(), Line);
+
+  return TargetRegionEntryInfo(ParentName, ID.getDevice(), ID.getFile(),
+                               std::get<1>(FileIDInfo));
 }
 
 Constant *OpenMPIRBuilder::getAddrOfDeclareTargetVar(