[Mlir-commits] [mlir] [MLIR][LLVM] Refactor globals insertion point in import (PR #127490)

[Henrich Lauko]

https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/127490

>From 5fdd3641dcab2f9b92c191bf365dfa6dc50e4391 Mon Sep 17 00:00:00 2001
From: xlauko <xlauko at mail.muni.cz>
Date: Mon, 17 Feb 2025 14:31:25 +0100
Subject: [PATCH] [MLIR][LLVM] Refactor globals insertion point translation
 (NFC)

---
 .../include/mlir/Target/LLVMIR/ModuleImport.h |  6 ++--
 mlir/lib/Target/LLVMIR/ModuleImport.cpp       | 30 +++++++++----------
 mlir/test/Target/LLVMIR/Import/alias.ll       | 18 ++++++-----
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 4642d58760ca8..6c673295d8dcc 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -407,6 +407,10 @@ class ModuleImport {
   /// always requires a symbol name.
   FlatSymbolRefAttr
   getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar);
+  /// Returns the global insertion point for the next global operation. If the
+  /// `globalInsertionOp` is set, the insertion point is placed after the
+  /// specified operation. Otherwise, it defaults to the start of the module.
+  OpBuilder::InsertionGuard setGlobalInsertionPoint();
 
   /// Builder pointing at where the next instruction should be generated.
   OpBuilder builder;
@@ -416,8 +420,6 @@ class ModuleImport {
   Operation *constantInsertionOp = nullptr;
   /// Operation to insert the next global after.
   Operation *globalInsertionOp = nullptr;
-  /// Operation to insert the next alias after.
-  Operation *aliasInsertionOp = nullptr;
   /// Operation to insert comdat selector operations into.
   ComdatOp globalComdatOp = nullptr;
   /// The current context.
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index fd0283b856b6b..8445e609c2244 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -962,13 +962,18 @@ ModuleImport::getOrCreateNamelessSymbolName(llvm::GlobalVariable *globalVar) {
   return symbolRef;
 }
 
-LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) {
-  // Insert the global after the last one or at the start of the module.
+OpBuilder::InsertionGuard ModuleImport::setGlobalInsertionPoint() {
   OpBuilder::InsertionGuard guard(builder);
-  if (!aliasInsertionOp)
-    builder.setInsertionPointToStart(mlirModule.getBody());
+  if (globalInsertionOp)
+    builder.setInsertionPointAfter(globalInsertionOp);
   else
-    builder.setInsertionPointAfter(aliasInsertionOp);
+    builder.setInsertionPointToStart(mlirModule.getBody());
+  return guard;
+}
+
+LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) {
+  // Insert the alias after the last one or at the start of the module.
+  OpBuilder::InsertionGuard guard = setGlobalInsertionPoint();
 
   Type type = convertType(alias->getValueType());
   AliasOp aliasOp = builder.create<AliasOp>(
@@ -977,7 +982,7 @@ LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) {
       /*dso_local=*/alias->isDSOLocal(),
       /*thread_local=*/alias->isThreadLocal(),
       /*attrs=*/ArrayRef<NamedAttribute>());
-  aliasInsertionOp = aliasOp;
+  globalInsertionOp = aliasOp;
 
   clearRegionState();
   Block *block = builder.createBlock(&aliasOp.getInitializerRegion());
@@ -996,11 +1001,7 @@ LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) {
 
 LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
   // Insert the global after the last one or at the start of the module.
-  OpBuilder::InsertionGuard guard(builder);
-  if (!globalInsertionOp)
-    builder.setInsertionPointToStart(mlirModule.getBody());
-  else
-    builder.setInsertionPointAfter(globalInsertionOp);
+  OpBuilder::InsertionGuard guard = setGlobalInsertionPoint();
 
   Attribute valueAttr;
   if (globalVar->hasInitializer())
@@ -1096,11 +1097,8 @@ ModuleImport::convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar) {
     priorities.push_back(priority->getValue().getZExtValue());
   }
 
-  OpBuilder::InsertionGuard guard(builder);
-  if (!globalInsertionOp)
-    builder.setInsertionPointToStart(mlirModule.getBody());
-  else
-    builder.setInsertionPointAfter(globalInsertionOp);
+  // Insert the global after the last one or at the start of the module.
+  OpBuilder::InsertionGuard guard = setGlobalInsertionPoint();
 
   if (globalVar->getName() == getGlobalCtorsVarName()) {
     globalInsertionOp = builder.create<LLVM::GlobalCtorsOp>(
diff --git a/mlir/test/Target/LLVMIR/Import/alias.ll b/mlir/test/Target/LLVMIR/Import/alias.ll
index 3ab68a7d8fb81..23eaecb9c9fa7 100644
--- a/mlir/test/Target/LLVMIR/Import/alias.ll
+++ b/mlir/test/Target/LLVMIR/Import/alias.ll
@@ -68,14 +68,6 @@ entry:
 @a1 = private alias i32, ptr @g1
 @a2 = private alias ptr, ptr @a1
 
-; CHECK: llvm.mlir.alias private @a1 {dso_local} : i32 {
-; CHECK:   %[[ADDR:.*]] = llvm.mlir.addressof @g1 : !llvm.ptr
-; CHECK:   llvm.return %[[ADDR]] : !llvm.ptr
-; CHECK: }
-; CHECK: llvm.mlir.alias private @a2 {dso_local} : !llvm.ptr {
-; CHECK-NEXT:   %[[ADDR:.*]] = llvm.mlir.addressof @a1 : !llvm.ptr
-; CHECK-NEXT:   llvm.return %[[ADDR]] : !llvm.ptr
-; CHECK-NEXT: }
 
 ; CHECK: llvm.mlir.global internal constant @g2() {addr_space = 0 : i32, dso_local} : !llvm.ptr {
 ; CHECK-NEXT:   %[[ADDR:.*]] = llvm.mlir.addressof @a1 : !llvm.ptr
@@ -86,3 +78,13 @@ entry:
 ; CHECK-NEXT:   %[[ADDR:.*]] = llvm.mlir.addressof @a2 : !llvm.ptr
 ; CHECK-NEXT:   llvm.return %[[ADDR]] : !llvm.ptr
 ; CHECK-NEXT: }
+
+; CHECK: llvm.mlir.alias private @a1 {dso_local} : i32 {
+; CHECK-NEXT:   %[[ADDR:.*]] = llvm.mlir.addressof @g1 : !llvm.ptr
+; CHECK-NEXT:   llvm.return %[[ADDR]] : !llvm.ptr
+; CHECK-NEXT: }
+
+; CHECK: llvm.mlir.alias private @a2 {dso_local} : !llvm.ptr {
+; CHECK-NEXT:   %[[ADDR:.*]] = llvm.mlir.addressof @a1 : !llvm.ptr
+; CHECK-NEXT:   llvm.return %[[ADDR]] : !llvm.ptr
+; CHECK-NEXT: }