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

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Christian Ulmann (Dinistro)

<details>
<summary>Changes</summary>

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`.

---
Full diff: https://github.com/llvm/llvm-project/pull/111703.diff


2 Files Affected:

- (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+11-6) 
- (modified) mlir/test/Target/LLVMIR/Import/global-variables.ll (+19) 


``````````diff
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

``````````

</details>


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