[MLIR][LLVM] Implement LLVM dialect support for global aliases #125295

bcardosolopes · 2025-01-31T21:17:54Z

This includes support for module translation and module import and add tests for both.

Fix #115390
ClangIR cannot currently lower global aliases to LLVM because of missing support for this.

This includes support for module translation and module import and add tests for both. ClangIR cannot currently lower global aliases to LLVM because of missing support for this.

llvmbot · 2025-01-31T21:20:19Z

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Bruno Cardoso Lopes (bcardosolopes)

Changes

This includes support for module translation and module import and add tests for both.

Fix #115390
ClangIR cannot currently lower global aliases to LLVM because of missing support for this.

Patch is 25.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125295.diff

8 Files Affected:

(modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+86)
(modified) mlir/include/mlir/Target/LLVMIR/ModuleImport.h (+8)
(modified) mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h (+11)
(modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+172)
(modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+59-1)
(modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+66-2)
(added) mlir/test/Target/LLVMIR/Import/alias.ll (+45)
(added) mlir/test/Target/LLVMIR/alias.mlir (+49)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index b2281536aa40b6..6687d31e326520 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1443,6 +1443,92 @@ def LLVM_GlobalDtorsOp : LLVM_Op<"mlir.global_dtors", [
   let hasVerifier = 1;
 }
 
+def LLVM_AliasOp : LLVM_Op<"mlir.alias",
+    [IsolatedFromAbove, SingleBlockImplicitTerminator<"ReturnOp">, Symbol]> {
+  let arguments = (ins
+    TypeAttr:$alias_type,
+    StrAttr:$sym_name,
+    Linkage:$linkage,
+    UnitAttr:$dso_local,
+    UnitAttr:$thread_local_,
+    UnitAttr:$externally_initialized,
+    DefaultValuedAttr<ConfinedAttr<I32Attr, [IntNonNegative]>, "0">:$addr_space,
+    OptionalAttr<UnnamedAddr>:$unnamed_addr,
+    OptionalAttr<StrAttr>:$section,
+    OptionalAttr<SymbolRefAttr>:$comdat,
+    DefaultValuedAttr<Visibility, "mlir::LLVM::Visibility::Default">:$visibility_
+  );
+  let summary = "LLVM dialect alias.";
+  let description = [{
+    `llvm.mlir.alias` is a top level operation that defines a global alias for
+    global variables and functions. The operation is always initialized by
+    using a initializer region which could be a direct map to another global
+    value or contain some address computation on top of it.
+
+    It uses an @-identifier for its value, which will be uniqued by the module
+    with respect to other @-identifiers in it.
+
+    Similarly to functions and globals, they can also have a linkage attribute.
+    This attribute is placed between `llvm.mlir.alias` and the symbol name. If
+    the attribute is omitted, `external` linkage is assumed by default.
+
+    Examples:
+
+    ```mlir
+    // Global alias use @-identifiers.
+    llvm.mlir.alias external @foo_alias {addr_space = 0 : i32} : !llvm.ptr {
+      %0 = llvm.mlir.addressof @some_function : !llvm.ptr
+      llvm.return %0 : !llvm.ptr
+    }
+
+    // More complex initialization.
+    llvm.mlir.alias linkonce_odr hidden @glob
+    {addr_space = 0 : i32, dso_local} : !llvm.array<32 x i32> {
+      %0 = llvm.mlir.constant(1234 : i64) : i64
+      %1 = llvm.mlir.addressof @glob.private : !llvm.ptr
+      %2 = llvm.ptrtoint %1 : !llvm.ptr to i64
+      %3 = llvm.add %2, %0 : i64
+      %4 = llvm.inttoptr %3 : i64 to !llvm.ptr
+      llvm.return %4 : !llvm.ptr
+    }
+    ```
+  }];
+  let regions = (region AnyRegion:$initializer);
+
+  let builders = [
+    OpBuilder<(ins "Type":$type, "Linkage":$linkage,
+      "StringRef":$name,
+      CArg<"unsigned", "0">:$addrSpace,
+      CArg<"bool", "false">:$dsoLocal,
+      CArg<"bool", "false">:$thread_local_,
+      CArg<"SymbolRefAttr", "{}">:$comdat,
+      CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs,
+      CArg<"ArrayRef<Attribute>", "{}">:$dbgExprs)>
+  ];
+
+  let extraClassDeclaration = [{
+    /// Return the LLVM type of the global alias.
+    Type getType() {
+      return getAliasType();
+    }
+    /// Return the initializer region. This may be empty, but if it is not it
+    /// terminates in an `llvm.return` op with the initializer value.
+    Region &getInitializerRegion() {
+      return getOperation()->getRegion(0);
+    }
+    /// Return the initializer block. The initializer region always exist
+    /// (differently from llvm.global) and it terminates with an `llvm.return`
+    /// op with the aliasee value.
+    Block *getInitializerBlock() {
+      return &getInitializerRegion().front();
+    }
+  }];
+
+  let hasCustomAssemblyFormat = 1;
+  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
+}
+
 def LLVM_ComdatSelectorOp : LLVM_Op<"comdat_selector", [Symbol]> {
   let arguments = (ins
     SymbolNameAttr:$sym_name,
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 80ae4d679624c2..d4032c6bc4356b 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -67,6 +67,9 @@ class ModuleImport {
   /// Converts all global variables of the LLVM module to MLIR global variables.
   LogicalResult convertGlobals();
 
+  /// Converts all aliases of the LLVM module to MLIR variables.
+  LogicalResult convertAliases();
+
   /// Converts the data layout of the LLVM module to an MLIR data layout
   /// specification.
   LogicalResult convertDataLayout();
@@ -288,6 +291,9 @@ class ModuleImport {
   LogicalResult convertGlobal(llvm::GlobalVariable *globalVar);
   /// Imports the magic globals "global_ctors" and "global_dtors".
   LogicalResult convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar);
+  /// Converts an LLVM global alias variable into an MLIR LLVM dialect alias
+  /// operation if a conversion exists. Otherwise, returns failure.
+  LogicalResult convertAlias(llvm::GlobalAlias *alias);
   /// Returns personality of `func` as a FlatSymbolRefAttr.
   FlatSymbolRefAttr getPersonalityAsAttr(llvm::Function *func);
   /// Imports `bb` into `block`, which must be initially empty.
@@ -406,6 +412,8 @@ 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/include/mlir/Target/LLVMIR/ModuleTranslation.h b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
index 1b62437761ed9d..28bc3642eb73dc 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -188,6 +188,12 @@ class ModuleTranslation {
     return globalsMapping.lookup(op);
   }
 
+  /// Finds an LLVM IR global value that corresponds to the given MLIR operation
+  /// defining a global alias value.
+  llvm::GlobalValue *lookupAlias(Operation *op) {
+    return aliasesMapping.lookup(op);
+  }
+
   /// Returns the OpenMP IR builder associated with the LLVM IR module being
   /// constructed.
   llvm::OpenMPIRBuilder *getOpenMPBuilder();
@@ -322,6 +328,7 @@ class ModuleTranslation {
   LogicalResult convertFunctions();
   LogicalResult convertComdats();
   LogicalResult convertGlobals();
+  LogicalResult convertAliases();
   LogicalResult convertOneFunction(LLVMFuncOp func);
   LogicalResult convertBlockImpl(Block &bb, bool ignoreArguments,
                                  llvm::IRBuilderBase &builder,
@@ -366,6 +373,10 @@ class ModuleTranslation {
   /// Mappings between llvm.mlir.global definitions and corresponding globals.
   DenseMap<Operation *, llvm::GlobalValue *> globalsMapping;
 
+  /// Mappings between llvm.mlir.alias definitions and corresponding global
+  /// aliases.
+  DenseMap<Operation *, llvm::GlobalValue *> aliasesMapping;
+
   /// A stateful object used to translate types.
   TypeToLLVMIRTranslator typeTranslator;
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index ef5f1b069b40a3..10ed998a808c86 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2422,6 +2422,178 @@ LogicalResult GlobalDtorsOp::verify() {
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// Builder, printer and verifier for LLVM::GlobalOp.
+//===----------------------------------------------------------------------===//
+
+void AliasOp::build(OpBuilder &builder, OperationState &result, Type type,
+                    Linkage linkage, StringRef name, unsigned addrSpace,
+                    bool dsoLocal, bool threadLocal, SymbolRefAttr comdat,
+                    ArrayRef<NamedAttribute> attrs,
+                    ArrayRef<Attribute> dbgExprs) {
+  result.addAttribute(getSymNameAttrName(result.name),
+                      builder.getStringAttr(name));
+  result.addAttribute(getAliasTypeAttrName(result.name), TypeAttr::get(type));
+  if (dsoLocal)
+    result.addAttribute(getDsoLocalAttrName(result.name),
+                        builder.getUnitAttr());
+  if (threadLocal)
+    result.addAttribute(getThreadLocal_AttrName(result.name),
+                        builder.getUnitAttr());
+  if (comdat)
+    result.addAttribute(getComdatAttrName(result.name), comdat);
+
+  result.addAttribute(getLinkageAttrName(result.name),
+                      LinkageAttr::get(builder.getContext(), linkage));
+  if (addrSpace != 0)
+    result.addAttribute(getAddrSpaceAttrName(result.name),
+                        builder.getI32IntegerAttr(addrSpace));
+  result.attributes.append(attrs.begin(), attrs.end());
+
+  result.addRegion();
+}
+
+void AliasOp::print(OpAsmPrinter &p) {
+  p << ' ' << stringifyLinkage(getLinkage()) << ' ';
+  StringRef visibility = stringifyVisibility(getVisibility_());
+  if (!visibility.empty())
+    p << visibility << ' ';
+  if (getThreadLocal_())
+    p << "thread_local ";
+  if (auto unnamedAddr = getUnnamedAddr()) {
+    StringRef str = stringifyUnnamedAddr(*unnamedAddr);
+    if (!str.empty())
+      p << str << ' ';
+  }
+  p.printSymbolName(getSymName());
+  if (auto comdat = getComdat())
+    p << " comdat(" << *comdat << ')';
+
+  // Note that the alignment attribute is printed using the
+  // default syntax here, even though it is an inherent attribute
+  // (as defined in https://mlir.llvm.org/docs/LangRef/#attributes)
+  p.printOptionalAttrDict((*this)->getAttrs(),
+                          {SymbolTable::getSymbolAttrName(),
+                           getAliasTypeAttrName(), getLinkageAttrName(),
+                           getUnnamedAddrAttrName(), getThreadLocal_AttrName(),
+                           getVisibility_AttrName(), getComdatAttrName(),
+                           getUnnamedAddrAttrName()});
+
+  // Print the trailing type
+  p << " : " << getType();
+
+  Region &initializer = getInitializerRegion();
+  if (!initializer.empty()) {
+    p << ' ';
+    p.printRegion(initializer, /*printEntryBlockArgs=*/false);
+  }
+}
+
+// operation ::= `llvm.mlir.alias` linkage? visibility?
+//               (`unnamed_addr` | `local_unnamed_addr`)?
+//               `thread_local`? `@` identifier
+//               `(` attribute? `)` (`comdat(` symbol-ref-id `)`)?
+//               attribute-list? (`:` type)? region
+//
+// The type can be omitted for string attributes, in which case it will be
+// inferred from the value of the string as [strlen(value) x i8].
+ParseResult AliasOp::parse(OpAsmParser &parser, OperationState &result) {
+  MLIRContext *ctx = parser.getContext();
+  // Parse optional linkage, default to External.
+  result.addAttribute(getLinkageAttrName(result.name),
+                      LLVM::LinkageAttr::get(
+                          ctx, parseOptionalLLVMKeyword<Linkage>(
+                                   parser, result, LLVM::Linkage::External)));
+
+  // Parse optional visibility, default to Default.
+  result.addAttribute(getVisibility_AttrName(result.name),
+                      parser.getBuilder().getI64IntegerAttr(
+                          parseOptionalLLVMKeyword<LLVM::Visibility, int64_t>(
+                              parser, result, LLVM::Visibility::Default)));
+
+  // Parse optional UnnamedAddr, default to None.
+  result.addAttribute(getUnnamedAddrAttrName(result.name),
+                      parser.getBuilder().getI64IntegerAttr(
+                          parseOptionalLLVMKeyword<UnnamedAddr, int64_t>(
+                              parser, result, LLVM::UnnamedAddr::None)));
+
+  if (succeeded(parser.parseOptionalKeyword("thread_local")))
+    result.addAttribute(getThreadLocal_AttrName(result.name),
+                        parser.getBuilder().getUnitAttr());
+
+  StringAttr name;
+  if (parser.parseSymbolName(name, getSymNameAttrName(result.name),
+                             result.attributes))
+    return failure();
+
+  if (succeeded(parser.parseOptionalKeyword("comdat"))) {
+    SymbolRefAttr comdat;
+    if (parser.parseLParen() || parser.parseAttribute(comdat) ||
+        parser.parseRParen())
+      return failure();
+
+    result.addAttribute(getComdatAttrName(result.name), comdat);
+  }
+
+  SmallVector<Type, 1> types;
+  if (parser.parseOptionalAttrDict(result.attributes) ||
+      parser.parseOptionalColonTypeList(types))
+    return failure();
+
+  if (types.size() > 1)
+    return parser.emitError(parser.getNameLoc(), "expected zero or one type");
+
+  Region &initRegion = *result.addRegion();
+  if (parser.parseRegion(initRegion).failed())
+    return failure();
+
+  result.addAttribute(getAliasTypeAttrName(result.name),
+                      TypeAttr::get(types[0]));
+  return success();
+}
+
+LogicalResult AliasOp::verify() {
+  bool validType = isCompatibleOuterType(getType())
+                       ? !llvm::isa<LLVMVoidType, LLVMTokenType,
+                                    LLVMMetadataType, LLVMLabelType>(getType())
+                       : llvm::isa<PointerElementTypeInterface>(getType());
+  if (!validType)
+    return emitOpError(
+        "expects type to be a valid element type for an LLVM global");
+  if ((*this)->getParentOp() && !satisfiesLLVMModule((*this)->getParentOp()))
+    return emitOpError("must appear at the module level");
+
+  if (getLinkage() == Linkage::Appending) {
+    if (!llvm::isa<LLVMArrayType>(getType())) {
+      return emitOpError() << "expected array type for '"
+                           << stringifyLinkage(Linkage::Appending)
+                           << "' linkage";
+    }
+  }
+
+  if (failed(verifyComdat(*this, getComdat())))
+    return failure();
+
+  return success();
+}
+
+LogicalResult AliasOp::verifyRegions() {
+  if (Block *b = getInitializerBlock()) {
+    ReturnOp ret = cast<ReturnOp>(b->getTerminator());
+    if (ret.operand_type_begin() == ret.operand_type_end())
+      return emitOpError("initializer region cannot return void");
+
+    for (Operation &op : *b) {
+      auto iface = dyn_cast<MemoryEffectOpInterface>(op);
+      if (!iface || !iface.hasNoEffect())
+        return op.emitError()
+               << "ops with side effects not allowed in aliases initializers";
+    }
+  }
+
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // ShuffleVectorOp
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 5ebde22cccbdf3..7c8b66b06d1715 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -647,6 +647,16 @@ LogicalResult ModuleImport::convertGlobals() {
   return success();
 }
 
+LogicalResult ModuleImport::convertAliases() {
+  for (llvm::GlobalAlias &alias : llvmModule->aliases()) {
+    if (failed(convertAlias(&alias))) {
+      return emitError(UnknownLoc::get(context))
+             << "unhandled global alias: " << diag(alias);
+    }
+  }
+  return success();
+}
+
 LogicalResult ModuleImport::convertDataLayout() {
   Location loc = mlirModule.getLoc();
   DataLayoutImporter dataLayoutImporter(context, llvmModule->getDataLayout());
@@ -952,6 +962,53 @@ 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 guard(builder);
+  if (!aliasInsertionOp)
+    builder.setInsertionPointToStart(mlirModule.getBody());
+  else
+    builder.setInsertionPointAfter(aliasInsertionOp);
+
+  Type type = convertType(alias->getValueType());
+
+  // Workaround to support LLVM's nameless globals. MLIR, in contrast to LLVM,
+  // always requires a symbol name.
+  StringRef aliasName = alias->getName();
+  if (aliasName.empty())
+    return emitError(UnknownLoc::get(builder.getContext()))
+           << "expects valid name";
+
+  AliasOp aliasOp = builder.create<AliasOp>(
+      mlirModule.getLoc(), type, convertLinkageFromLLVM(alias->getLinkage()),
+      StringRef(aliasName),
+      /*addr_space=*/alias->getAddressSpace(),
+      /*dso_local=*/alias->isDSOLocal(),
+      /*thread_local=*/alias->isThreadLocal(), /*comdat=*/SymbolRefAttr(),
+      /*attrs=*/ArrayRef<NamedAttribute>());
+  aliasInsertionOp = aliasOp;
+
+  clearRegionState();
+  Block *block = builder.createBlock(&aliasOp.getInitializerRegion());
+  setConstantInsertionPointToStart(block);
+  FailureOr<Value> initializer = convertConstantExpr(alias->getAliasee());
+  if (failed(initializer))
+    return failure();
+  builder.create<ReturnOp>(aliasOp.getLoc(), *initializer);
+
+  if (alias->hasAtLeastLocalUnnamedAddr()) {
+    aliasOp.setUnnamedAddr(convertUnnamedAddrFromLLVM(alias->getUnnamedAddr()));
+  }
+  if (alias->hasSection())
+    aliasOp.setSection(alias->getSection());
+  aliasOp.setVisibility_(convertVisibilityFromLLVM(alias->getVisibility()));
+
+  if (alias->hasComdat())
+    aliasOp.setComdatAttr(comdatMapping.lookup(alias->getComdat()));
+
+  return success();
+}
+
 LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
   // Insert the global after the last one or at the start of the module.
   OpBuilder::InsertionGuard guard(builder);
@@ -2456,6 +2513,7 @@ mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
     return {};
   if (failed(moduleImport.convertFunctions()))
     return {};
-  moduleImport.convertTargetTriple();
+  if (failed(moduleImport.convertAliases()))
+    return {};
   return module;
 }
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 4367100e3aca68..70b475d83bbe99 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -746,6 +746,8 @@ void ModuleTranslation::forgetMapping(Region &region) {
           branchMapping.erase(&op);
         if (isa<LLVM::GlobalOp>(op))
           globalsMapping.erase(&op);
+        if (isa<LLVM::AliasOp>(op))
+          aliasesMapping.erase(&op);
         if (isa<LLVM::CallOp>(op))
           callMapping.erase(&op);
         llvm::append_range(
@@ -1251,6 +1253,66 @@ LogicalResult ModuleTranslation::convertGlobals() {
   return success();
 }
 
+/// Convert aliases.
+LogicalResult ModuleTranslation::convertAliases() {
+  for (auto op : getModuleBody(mlirModule).getOps<LLVM::AliasOp>()) {
+    llvm::Type *type = convertType(op.getType());
+    llvm::Constant *cst = nullptr;
+    auto linkage = convertLinkageToLLVM(op.getLinkage());
+    llvm::Module &llvmMod = *llvmModule;
+
+    llvm::GlobalAlias *var = llvm::GlobalAlias::create(
+        type, op.getAddrSpace(), linkage, op.getSymName(), cst, &llvmMod);
+
+    var->setThreadLocalMode(op.getThreadLocal_()
+                                ? llvm::GlobalAlias::GeneralDynamicTLSModel
+                                : llvm::GlobalAlias::NotThreadLocal);
+
+    // Note there is no need to steup the comdat because GlobalAlias calls into
+    // the aliasee comdat information automatically.
+
+    if (op.getUnnamedAddr().has_value())
+      var->setUnnamedAddr(convertUnnamedAddrToLLVM(*op.getUnnamedAddr()));
+
+    var->setVisibility(convertVisibilityToLLVM(op.getVisibility_()));
+
+    aliasesMapping.try_emplace(op, var);
+  }
+
+  // Convert global aliases. This is done after all global aliases
+  // have been created in LLVM IR because a global body may refer to another
+  // global alias. So all aliases need to be mapped first.
+  for (auto op : getModuleBody(mlirModule).getOps<LLVM::AliasOp>()) {
+    if (Block *initializer = op.getInitializerBlock()) {
+      llvm::IRBuilder<> builder(llvmModule->getContext());
+
+      for (auto &op : initializer->without_terminator()) {
+        if (failed(convertOperation(op, builder)))
+          return emitError(op.getLoc(), "fail to convert alias initializer");
+        auto *cst = dyn_cast<llvm::Constant>(lookupValue(op.getResult(0)));
+        if (!cst)
+          return emitError(op.getLoc(), "unemittable constant value");
+      }
+
+      ReturnOp ret = cast<ReturnOp>(initializer->getTerminator());
+      llvm::Constant *cst =
+          cast<llvm::Constant>(lookupValue(ret.getOperand(0)));
+      assert(aliasesMapping.count(op));
+      auto *alias = cast<llvm::GlobalAlias>(aliasesMapping...
[truncated]

github-actions · 2025-01-31T21:27:30Z

✅ With the latest revision this PR passed the undef deprecator.

bcardosolopes · 2025-02-01T00:59:59Z

CI failures seem unrelated to this PR (all from Import/target-triple.ll)

gysit

Thanks for adding aliases!

I did leave a first round of comments. I wonder if really all of the arguments have to be on the alias. In the language reference I see actually less https://llvm.org/docs/LangRef.html#aliases.

mlir/lib/Target/LLVMIR/ModuleImport.cpp

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

mlir/lib/Target/LLVMIR/ModuleImport.cpp

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

mlir/test/Target/LLVMIR/Import/alias.ll

Dinistro

Dropped a few comments. This is also missing a roundtrip test for the operation in the LLVM dialect

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

mlir/test/Target/LLVMIR/Import/alias.ll

mlir/test/Target/LLVMIR/alias.mlir

bcardosolopes · 2025-02-03T23:22:31Z

Hi @gysit, thanks a bunch for the review.

I wonder if really all of the arguments have to be on the alias. In the language reference I see actually less https://llvm.org/docs/LangRef.html#aliases.

I removed the copy-pasta ones, addrspace and dso_local don't show up in the docs, I see both in LLVM tests but lacking representation in the docs

bcardosolopes · 2025-02-04T00:00:12Z

@Dinistro thanks for the review, added roundtrip tests and refactor parse (to the extend as to not abuse much on templates)

…ce each other

… and add more testcases

bcardosolopes · 2025-02-04T04:01:22Z

I believe I addressed all reviews, let me know if there's more :)

Dinistro

Thanks for addressing the comments. I now had time to perform a more in-depth review and thus added more nit and style comments.

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

mlir/test/Target/LLVMIR/alias.mlir

mlir/test/Target/LLVMIR/Import/alias.ll

mlir/test/Target/LLVMIR/alias.mlir

mlir/test/Target/LLVMIR/Import/alias.ll

bcardosolopes · 2025-02-05T01:32:31Z

Round of review addressed, pending question related to SingleBlockWithTerminator.

gysit

Thanks for all the effort!

I had some last small comments but it is looking good.

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

mlir/lib/Target/LLVMIR/ModuleImport.cpp

mlir/test/Dialect/LLVMIR/alias.mlir

bcardosolopes · 2025-02-05T19:54:47Z

@gysit addressed all comments

gysit

Thanks!

LGTM modulo last nit.

mlir/test/Dialect/LLVMIR/alias.mlir

…125295) This includes support for module translation, module import and add tests for both. Fix llvm#115390 ClangIR cannot currently lower global aliases to LLVM because of missing support for this.

[MLIR][LLVM] Implement LLVM dialect support for global aliases

cff1cd3

This includes support for module translation and module import and add tests for both. ClangIR cannot currently lower global aliases to LLVM because of missing support for this.

bcardosolopes requested a review from gysit January 31, 2025 21:17

llvmbot added mlir:llvm mlir labels Jan 31, 2025

bcardosolopes requested a review from rolfmorel January 31, 2025 21:18

bcardosolopes added 2 commits January 31, 2025 13:28

Fix comments

8ccafc9

Remove undef from testcase

e36ff90

gysit reviewed Feb 1, 2025

View reviewed changes

Address Tobias review

f12825e

Dinistro reviewed Feb 3, 2025

View reviewed changes

bcardosolopes added 3 commits February 3, 2025 14:21

Add addrspace related tests

1186559

Reorganize tests a bit, use -split-input-file

1927cb5

More verification love

7b27f3a

bcardosolopes added 2 commits February 3, 2025 15:53

Refactor common parsing bits for GlobalOp and AliasOp

82ca97a

Add roundtrip tests for the parsing/printing

8711298

bcardosolopes added 5 commits February 3, 2025 16:25

Fix more nits and align linkage verification with LLVM

3eb7a53

Fix one more comment

f159dcc

Update parse comment

cf984ad

Handle conversion for globals/aliases together since they can referen…

77661e7

…ce each other

Teach LLVM_AddressOfOp to work with alias symbols, fix remaining bugs…

b669463

… and add more testcases

Dinistro requested review from Dinistro and gysit February 4, 2025 06:27

Dinistro reviewed Feb 4, 2025

View reviewed changes

Improve LLVM_AddressOfOp description doc

644424d

bcardosolopes added 5 commits February 4, 2025 11:26

More tablegen fixes, new test file in Dialect

0d181ee

Address more review comments

3932b16

More split-input-file love

d8bd0ab

Fix multiple printing of addressofs in init bodies

8683355

Get rid of address space as top level AliasOp attribute

d69436c

Fix test, removed too much from globals

f0755a4

gysit reviewed Feb 5, 2025

View reviewed changes

bcardosolopes added 3 commits February 5, 2025 11:27

Add one more verifier check to addressof

e4346f6

address more reviews

334d643

Remove addr_space from globals to clean it up a bit

bd0b538

gysit approved these changes Feb 5, 2025

View reviewed changes

mlir/test/Dialect/LLVMIR/alias.mlir Outdated Show resolved Hide resolved

Fix newline

7c293b2

bcardosolopes merged commit 4fb96f2 into llvm:main Feb 6, 2025
8 checks passed

bcardosolopes mentioned this pull request Feb 10, 2025

Missing function pointer (e.g. void (ptr)) support in mlir::LLVM::GlobalOp #115392

Closed

[MLIR][LLVM] Implement LLVM dialect support for global aliases #125295

[MLIR][LLVM] Implement LLVM dialect support for global aliases #125295

Uh oh!

Conversation

bcardosolopes commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcardosolopes commented Feb 1, 2025

Uh oh!

gysit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dinistro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes commented Feb 3, 2025

Uh oh!

bcardosolopes commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcardosolopes commented Feb 4, 2025

Uh oh!

Dinistro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes commented Feb 5, 2025

Uh oh!

gysit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes commented Feb 5, 2025

Uh oh!

gysit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcardosolopes commented Jan 31, 2025 •

edited

Loading

llvmbot commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading

bcardosolopes commented Feb 4, 2025 •

edited

Loading