[Mlir-commits] [mlir] [MLIR][LLVM] Fix import of globals with references to other globals (PR #111703)

[Christian Ulmann]

https://github.com/Dinistro created https://github.com/llvm/llvm-project/pull/111703

This commit addresses an issue with importing globals that reference other globals. This case did not properly work due to not considering that `llvm::GlobalVariables` are derived from `llvm::Constant`.

>From 14e4418572ff2f381f0ee3e6e90c08120bf7fad6 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 9 Oct 2024 15:43:22 +0000
Subject: [PATCH] [MLIR][LLVM] Fix import of globals with references to other
 globals

This commit addresses an issue with importing globals that reference
other globals. This case did not properly work due to not considering
that `llvm::GlobalVariables` are derived from `llvm::Constant`.
---
 mlir/lib/Target/LLVMIR/ModuleImport.cpp       | 17 +++++++++++------
 .../Target/LLVMIR/Import/global-variables.ll  | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 21f2050cbceb9c..a26cbdae561373 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -792,10 +792,6 @@ Attribute ModuleImport::getConstantAsAttr(llvm::Constant *constant) {
   if (Attribute scalarAttr = getScalarConstantAsAttr(builder, constant))
     return scalarAttr;
 
-  // Convert function references.
-  if (auto *func = dyn_cast<llvm::Function>(constant))
-    return SymbolRefAttr::get(builder.getContext(), func->getName());
-
   // Returns the static shape of the provided type if possible.
   auto getConstantShape = [&](llvm::Type *type) {
     return llvm::dyn_cast_if_present<ShapedType>(
@@ -1019,6 +1015,14 @@ ModuleImport::getConstantsToConvert(llvm::Constant *constant) {
   workList.insert(constant);
   while (!workList.empty()) {
     llvm::Constant *current = workList.back();
+    // References of global objects are just pointers to the object. Avoid
+    // walking the elements of these here.
+    if (isa<llvm::GlobalObject>(current)) {
+      orderedSet.insert(current);
+      workList.pop_back();
+      continue;
+    }
+
     // Collect all dependencies of the current constant and add them to the
     // adjacency list if none has been computed before.
     auto [adjacencyIt, inserted] = adjacencyLists.try_emplace(current);
@@ -1096,12 +1100,13 @@ FailureOr<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
   }
 
   // Convert global variable accesses.
-  if (auto *globalVar = dyn_cast<llvm::GlobalVariable>(constant)) {
+  if (auto *globalVar = dyn_cast<llvm::GlobalObject>(constant)) {
     Type type = convertType(globalVar->getType());
     StringRef globalName = globalVar->getName();
     FlatSymbolRefAttr symbolRef;
+    // Empty names are only allowed for global variables.
     if (globalName.empty())
-      symbolRef = namelessGlobals[globalVar];
+      symbolRef = namelessGlobals[cast<llvm::GlobalVariable>(globalVar)];
     else
       symbolRef = FlatSymbolRefAttr::get(context, globalName);
     return builder.create<AddressOfOp>(loc, type, symbolRef).getResult();
diff --git a/mlir/test/Target/LLVMIR/Import/global-variables.ll b/mlir/test/Target/LLVMIR/Import/global-variables.ll
index cac7411431ae35..d6d2492b4f1770 100644
--- a/mlir/test/Target/LLVMIR/Import/global-variables.ll
+++ b/mlir/test/Target/LLVMIR/Import/global-variables.ll
@@ -41,6 +41,25 @@
 
 ; // -----
 
+; Verifies that converting a reference to a global does not convert the global
+; a second time.
+
+; CHECK-LABEL: llvm.mlir.global external constant @reference
+; CHECK-NEXT: %[[ADDR:.*]] = llvm.mlir.addressof @simple
+; CHECK-NEXT: llvm.return %[[ADDR]]
+ at reference = constant ptr @simple
+
+ at simple = global { ptr } { ptr null }
+
+; // -----
+
+; CHECK-LABEL: llvm.mlir.global external @recursive
+; CHECK: %[[ADDR:.*]] = llvm.mlir.addressof @recursive
+; CHECK: llvm.return %[[ADDR]]
+ at recursive = global ptr @recursive
+
+; // -----
+
 ; alignment attribute.
 
 ; CHECK:  llvm.mlir.global private @global_int_align_32