From 03b49b337506cd8f0fbbb3346e5ab8da7ffcb04a Mon Sep 17 00:00:00 2001 From: xlauko Date: Mon, 17 Feb 2025 14:31:25 +0100 Subject: [PATCH] [MLIR][LLVM] Refactor globals insertion point in import --- .../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( @@ -977,7 +982,7 @@ LogicalResult ModuleImport::convertAlias(llvm::GlobalAlias *alias) { /*dso_local=*/alias->isDSOLocal(), /*thread_local=*/alias->isThreadLocal(), /*attrs=*/ArrayRef()); - 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( 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: }