[mlir][llvm] Add comdat attribute to functions

[gysit]

Summary

This revision adds comdat support to functions. Additionally,
it ensures only comdats that have uses are imported/exported and
only non-empty global comdat operations are created.

[Dinistro]

LGTM, module NITs

mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h

346

NIT: Is it possible to limit the key type to be the ComdatOp?

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

1810–1814

NIT: This comment is still outdated. It only lists a limited subset of the parsed parts.

mlir/lib/Target/LLVMIR/ModuleImport.cpp

1838

NIT: is there a reason for this change?

[gysit]

Address comments.

mlir/lib/Target/LLVMIR/ModuleImport.cpp

1838

I wanted to have the comdats before the metadata. Apart from that I do not have a strong reason.

[gysit]

Rebase.

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

def LLVM_LLVMFuncOp : LLVM_Op<"func", [
   }];
 
   let arguments = (ins
     StrAttr:$sym_name,
     TypeAttrOf<LLVM_FunctionType>:$function_type,
     DefaultValuedAttr<Linkage, "Linkage::External">:$linkage,
     UnitAttr:$dso_local,
     DefaultValuedAttr<CConv, "CConv::C">:$CConv,
+    OptionalAttr<SymbolRefAttr>:$comdat,
     OptionalAttr<FlatSymbolRefAttr>:$personality,
     OptionalAttr<StrAttr>:$garbageCollector,
     OptionalAttr<ArrayAttr>:$passthrough,
     OptionalAttr<DictArrayAttr>:$arg_attrs,
     OptionalAttr<DictArrayAttr>:$res_attrs,
     OptionalAttr<I64Attr>:$function_entry_count,
     OptionalAttr<LLVM_MemoryEffectsAttr>:$memory,
     DefaultValuedAttr<Visibility, "mlir::LLVM::Visibility::Default">:$visibility_,
     OptionalAttr<UnitAttr>:$arm_streaming,
     OptionalAttr<UnitAttr>:$arm_locally_streaming
   );
 
   let regions = (region AnyRegion:$body);
 
   let skipDefaultBuilders = 1;
 
   let builders = [
     OpBuilder<(ins "StringRef":$name, "Type":$type,
       CArg<"Linkage", "Linkage::External">:$linkage,
       CArg<"bool", "false">:$dsoLocal,
       CArg<"CConv", "CConv::C">:$cconv,
+      CArg<"SymbolRefAttr", "{}">:$comdat,
       CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs,
       CArg<"ArrayRef<DictionaryAttr>", "{}">:$argAttrs,
       CArg<"std::optional<uint64_t>", "{}">:$functionEntryCount)>
   ];
 
   let extraClassDeclaration = [{
     // Add an entry block to an empty function, and set up the block arguments
     // to match the signature of the function.

mlir/include/mlir/Target/LLVMIR/ModuleImport.h

private:
   /// conversions succeed and failure otherwise.
   LogicalResult processAccessGroupMetadata(const llvm::MDNode *node);
   /// Converts all LLVM alias scopes and domains starting from `node` to MLIR
   /// alias scope and domain operations and stores a mapping from every nested
   /// alias scope or alias domain node to the symbol pointing to the translated
   /// operation. Returns success if all conversions succeed and failure
   /// otherwise.
   LogicalResult processAliasScopeMetadata(const llvm::MDNode *node);
+  /// Converts the given LLVM comdat struct to an MLIR comdat selector operation
+  /// and stores a mapping from the struct to the symbol pointing to the
+  /// translated operation.
+  void processComdat(const llvm::Comdat *comdat);
 
   /// Builder pointing at where the next instruction should be generated.
   OpBuilder builder;
   /// Block to insert the next constant into.
   Block *constantInsertionBlock = nullptr;
   /// Operation to insert the next constant after.
   Operation *constantInsertionOp = nullptr;
   /// Operation to insert the next global after.
private:
   /// return a result have a valueMapping entry instead.
   DenseMap<llvm::Instruction *, Operation *> noResultOpMapping;
   /// Mapping between LLVM alias scope and domain metadata nodes and symbol
   /// references to the LLVM dialect operations corresponding to these nodes.
   DenseMap<const llvm::MDNode *, SymbolRefAttr> aliasScopeMapping;
   /// Mapping between LLVM TBAA metadata nodes and symbol references to the LLVM
   /// dialect TBAA operations corresponding to these nodes.
   DenseMap<const llvm::MDNode *, SymbolRefAttr> tbaaMapping;
+  /// Mapping between LLVM comdat structs and symbol references to LLVM dialect
+  /// comdat selector operations corresponding to these structs.
+  DenseMap<const llvm::Comdat *, SymbolRefAttr> comdatMapping;
   /// The stateful type translator (contains named structs).
   LLVM::TypeFromLLVMIRTranslator typeTranslator;
   /// Stateful debug information importer.
   std::unique_ptr<detail::DebugImporter> debugImporter;
   /// Loop annotation importer.
   std::unique_ptr<detail::LoopAnnotationImporter> loopAnnotationImporter;
 };
 
 } // namespace LLVM
 } // namespace mlir
 
 #endif // MLIR_TARGET_LLVMIR_MODULEIMPORT_H

mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h

 
 namespace detail {
 class DebugTranslation;
 class LoopAnnotationTranslation;
 } // namespace detail
 
 class DINodeAttr;
 class LLVMFuncOp;
+class ComdatSelectorOp;
 
 /// Implementation class for module translation. Holds a reference to the module
 /// being translated, and the mappings between the original and the translated
 /// functions, basic blocks and values. It is practically easier to hold these
 /// mappings in one class since the conversion of control flow operations
 /// needs to look up block and function mappings.
 class ModuleTranslation {
   friend std::unique_ptr<llvm::Module>
private:
   /// Mapping from an alias scope metadata operation to its LLVM metadata.
   /// This map is populated on module entry.
   DenseMap<Operation *, llvm::MDNode *> aliasScopeMetadataMapping;
 
   /// Mapping from a tbaa metadata operation to its LLVM metadata.
   /// This map is populated on module entry.
   DenseMap<const Operation *, llvm::MDNode *> tbaaMetadataMapping;
 
+  /// Mapping from a comdat selector operation to its LLVM comdat struct.
+  /// This map is populated on module entry.
+  DenseMap<ComdatSelectorOp, llvm::Comdat *> comdatMapping;
+
   /// Stack of user-specified state elements, useful when translating operations
   /// with regions.
   SmallVector<std::unique_ptr<StackFrame>> stack;
 
   /// A cache for the symbol tables constructed during symbols lookup.
   SymbolTableCollection symbolTableCollection;
 };
 

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

static void wrapForExternalCallers(OpBuilder &rewriter, Location loc,
   // argument.
   if (resultStructType)
     prependEmptyArgAttr(rewriter, attributes, funcOp);
   filterFuncAttributes(
       funcOp, /*filterArgAndResAttrs=*/static_cast<bool>(resultStructType),
       attributes);
   auto wrapperFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
       loc, llvm::formatv("_mlir_ciface_{0}", funcOp.getName()).str(),
-      wrapperFuncType, LLVM::Linkage::External, /*dsoLocal*/ false,
-      /*cconv*/ LLVM::CConv::C, attributes);
+      wrapperFuncType, LLVM::Linkage::External, /*dsoLocal=*/false,
+      /*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr, attributes);
 
   OpBuilder::InsertionGuard guard(rewriter);
   rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock());
 
   SmallVector<Value, 8> args;
   size_t argOffset = resultStructType ? 1 : 0;
   for (auto [index, argType] : llvm::enumerate(type.getInputs())) {
     Value arg = wrapperFuncOp.getArgument(index + argOffset);
if (resultStructType)
     prependEmptyArgAttr(builder, attributes, funcOp);
   filterFuncAttributes(
       funcOp, /*filterArgAndResAttrs=*/static_cast<bool>(resultStructType),
       attributes);
 
   // Create the auxiliary function.
   auto wrapperFunc = builder.create<LLVM::LLVMFuncOp>(
       loc, llvm::formatv("_mlir_ciface_{0}", funcOp.getName()).str(),
-      wrapperType, LLVM::Linkage::External, /*dsoLocal*/ false,
-      /*cconv*/ LLVM::CConv::C, attributes);
+      wrapperType, LLVM::Linkage::External, /*dsoLocal=*/false,
+      /*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr, attributes);
 
   // The wrapper that we synthetize here should only be visible in this module.
   newFuncOp.setLinkage(LLVM::Linkage::Private);
   builder.setInsertionPointToStart(newFuncOp.addEntryBlock());
 
   // Get a ValueRange containing arguments.
   FunctionType type = funcOp.getFunctionType();
   SmallVector<Value, 8> args;
if (funcOp->hasAttr(readnoneAttrName)) {
       }
       memoryAttr = LLVM::MemoryEffectsAttr::get(rewriter.getContext(),
                                                 {LLVM::ModRefInfo::NoModRef,
                                                  LLVM::ModRefInfo::NoModRef,
                                                  LLVM::ModRefInfo::NoModRef});
     }
     auto newFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
         funcOp.getLoc(), funcOp.getName(), llvmType, linkage,
-        /*dsoLocal*/ false, /*cconv*/ LLVM::CConv::C, attributes);
+        /*dsoLocal=*/false, /*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr,
+        attributes);
     // If the memory attribute was created, add it to the function.
     if (memoryAttr)
       newFuncOp.setMemoryAttr(memoryAttr);
     rewriter.inlineRegionBefore(funcOp.getBody(), newFuncOp.getBody(),
                                 newFuncOp.end());
     if (failed(rewriter.convertRegionTypes(&newFuncOp.getBody(), *typeConverter,
                                            &result)))
       return nullptr;

mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp

GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp gpuFuncOp, OpAdaptor adaptor,
   }
   // Add a dialect specific kernel attribute in addition to GPU kernel
   // attribute. The former is necessary for further translation while the
   // latter is expected by gpu.launch_func.
   if (gpuFuncOp.isKernel())
     attributes.emplace_back(kernelAttributeName, rewriter.getUnitAttr());
   auto llvmFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
       gpuFuncOp.getLoc(), gpuFuncOp.getName(), funcType,
-      LLVM::Linkage::External, /*dsoLocal*/ false, /*cconv*/ LLVM::CConv::C,
-      attributes);
+      LLVM::Linkage::External, /*dsoLocal=*/false, /*cconv=*/LLVM::CConv::C,
+      /*comdat=*/nullptr, attributes);
 
   {
     // Insert operations that correspond to converted workgroup and private
     // memory attributions to the body of the function. This must operate on
     // the original function, before the body region is inlined in the new
     // function to maintain the relation between block arguments and the
     // parent operation that assigns their semantics.
     OpBuilder::InsertionGuard guard(rewriter);

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

static LogicalResult verifyComdat(Operation *op,
 
   auto *comdatSelector = SymbolTable::lookupNearestSymbolFrom(op, *attr);
   if (!isa_and_nonnull<ComdatSelectorOp>(comdatSelector))
     return op->emitError() << "expected comdat symbol";
 
   return success();
 }
 
-// operation ::= `llvm.mlir.global` linkage? `constant`? `@` identifier
-//               `(` attribute? `)` align? attribute-list? (`:` type)? region?
-// align     ::= `align` `=` UINT64
+// operation ::= `llvm.mlir.global` linkage? visibility?
+//               (`unnamed_addr` | `local_unnamed_addr`)?
+//               `thread_local`? `constant`? `@` 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 GlobalOp::parse(OpAsmParser &parser, OperationState &result) {
   MLIRContext *ctx = parser.getContext();
   // Parse optional linkage, default to External.
   result.addAttribute(getLinkageAttrName(result.name),
                       LLVM::LinkageAttr::get(
Block *LLVMFuncOp::addEntryBlock() {
   LLVMFunctionType type = getFunctionType();
   for (unsigned i = 0, e = type.getNumParams(); i < e; ++i)
     entry->addArgument(type.getParamType(i), getLoc());
   return entry;
 }
 
 void LLVMFuncOp::build(OpBuilder &builder, OperationState &result,
                        StringRef name, Type type, LLVM::Linkage linkage,
-                       bool dsoLocal, CConv cconv,
+                       bool dsoLocal, CConv cconv, SymbolRefAttr comdat,
                        ArrayRef<NamedAttribute> attrs,
                        ArrayRef<DictionaryAttr> argAttrs,
                        std::optional<uint64_t> functionEntryCount) {
   result.addRegion();
   result.addAttribute(SymbolTable::getSymbolAttrName(),
                       builder.getStringAttr(name));
   result.addAttribute(getFunctionTypeAttrName(result.name),
                       TypeAttr::get(type));
   result.addAttribute(getLinkageAttrName(result.name),
                       LinkageAttr::get(builder.getContext(), linkage));
   result.addAttribute(getCConvAttrName(result.name),
                       CConvAttr::get(builder.getContext(), cconv));
   result.attributes.append(attrs.begin(), attrs.end());
   if (dsoLocal)
     result.addAttribute(getDsoLocalAttrName(result.name),
                         builder.getUnitAttr());
+  if (comdat)
+    result.addAttribute(getComdatAttrName(result.name), comdat);
   if (functionEntryCount)
     result.addAttribute(getFunctionEntryCountAttrName(result.name),
                         builder.getI64IntegerAttr(functionEntryCount.value()));
   if (argAttrs.empty())
     return;
 
   assert(llvm::cast<LLVMFunctionType>(type).getNumParams() == argAttrs.size() &&
          "expected as many argument attribute lists as arguments");
buildLLVMFunctionType(OpAsmParser &parser, SMLoc loc, ArrayRef<Type> inputs,
   }
   return LLVMFunctionType::get(llvmOutput, llvmInputs,
                                variadicFlag.isVariadic());
 }
 
 // Parses an LLVM function.
 //
 // operation ::= `llvm.func` linkage? cconv? function-signature
-// function-attributes?
-//               function-body
+//                (`comdat(` symbol-ref-id `)`)?
+//                function-attributes?
+//                function-body
 //
 ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
   // Default to external linkage if no keyword is provided.
   result.addAttribute(
       getLinkageAttrName(result.name),
       LinkageAttr::get(parser.getContext(),
                        parseOptionalLLVMKeyword<Linkage>(
                            parser, result, LLVM::Linkage::External)));
ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
   auto type =
       buildLLVMFunctionType(parser, signatureLocation, argTypes, resultTypes,
                             function_interface_impl::VariadicFlag(isVariadic));
   if (!type)
     return failure();
   result.addAttribute(getFunctionTypeAttrName(result.name),
                       TypeAttr::get(type));
 
+  // Parse the optional comdat selector.
+  if (succeeded(parser.parseOptionalKeyword("comdat"))) {
+    SymbolRefAttr comdat;
+    if (parser.parseLParen() || parser.parseAttribute(comdat) ||
+        parser.parseRParen())
+      return failure();
+
+    result.addAttribute(getComdatAttrName(result.name), comdat);
+  }
+
   if (failed(parser.parseOptionalAttrDictWithKeyword(result.attributes)))
     return failure();
   function_interface_impl::addArgAndResultAttrs(
       parser.getBuilder(), result, entryArgs, resultAttrs,
       getArgAttrsAttrName(result.name), getResAttrsAttrName(result.name));
 
   auto *body = result.addRegion();
   OptionalParseResult parseResult =
for (unsigned i = 0, e = fnType.getNumParams(); i < e; ++i)
     argTypes.push_back(fnType.getParamType(i));
 
   Type returnType = fnType.getReturnType();
   if (!llvm::isa<LLVMVoidType>(returnType))
     resTypes.push_back(returnType);
 
   function_interface_impl::printFunctionSignature(p, *this, argTypes,
                                                   isVarArg(), resTypes);
+
+  // Print the optional comdat selector.
+  if (auto comdat = getComdat())
+    p << " comdat(" << *comdat << ')';
+
   function_interface_impl::printFunctionAttributes(
       p, *this,
       {getFunctionTypeAttrName(), getArgAttrsAttrName(), getResAttrsAttrName(),
-       getLinkageAttrName(), getCConvAttrName(), getVisibility_AttrName()});
+       getLinkageAttrName(), getCConvAttrName(), getVisibility_AttrName(),
+       getComdatAttrName()});
 
   // Print the body if this is not an external function.
   Region &body = getBody();
   if (!body.empty()) {
     p << ' ';
     p.printRegion(body, /*printEntryBlockArgs=*/false,
                   /*printBlockTerminators=*/true);
   }
 }
 
 // Verifies LLVM- and implementation-specific properties of the LLVM func Op:
 // - functions don't have 'common' linkage
 // - external functions have 'external' or 'extern_weak' linkage;
 // - vararg is (currently) only supported for external functions;
 LogicalResult LLVMFuncOp::verify() {
   if (getLinkage() == LLVM::Linkage::Common)
     return emitOpError() << "functions cannot have '"
                          << stringifyLinkage(LLVM::Linkage::Common)
                          << "' linkage";
 
+  if (failed(verifyComdat(*this, getComdat())))
+    return failure();
+
   if (isExternal()) {
     if (getLinkage() != LLVM::Linkage::External &&
         getLinkage() != LLVM::Linkage::ExternWeak)
       return emitOpError() << "external functions must have '"
                            << stringifyLinkage(LLVM::Linkage::External)
                            << "' or '"
                            << stringifyLinkage(LLVM::Linkage::ExternWeak)
                            << "' linkage";

mlir/lib/Target/LLVMIR/ModuleImport.cpp

 }
 
 MetadataOp ModuleImport::getGlobalMetadataOp() {
   if (globalMetadataOp)
     return globalMetadataOp;
 
   OpBuilder::InsertionGuard guard(builder);
   builder.setInsertionPointToEnd(mlirModule.getBody());
-  return globalMetadataOp = builder.create<MetadataOp>(
-             mlirModule.getLoc(), getGlobalMetadataOpName());
+  globalMetadataOp = builder.create<MetadataOp>(mlirModule.getLoc(),
+                                                getGlobalMetadataOpName());
+  globalInsertionOp = globalMetadataOp;
+  return globalMetadataOp;
 }
 
 ComdatOp ModuleImport::getGlobalComdatOp() {
   if (globalComdatOp)
     return globalComdatOp;
 
   OpBuilder::InsertionGuard guard(builder);
-  builder.setInsertionPointToStart(mlirModule.getBody());
-  return globalComdatOp = builder.create<ComdatOp>(mlirModule.getLoc(),
-                                                   getGlobalComdatOpName());
+  builder.setInsertionPointToEnd(mlirModule.getBody());
+  globalComdatOp =
+      builder.create<ComdatOp>(mlirModule.getLoc(), getGlobalComdatOpName());
+  globalInsertionOp = globalComdatOp;
+  return globalComdatOp;
 }
 
 LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
   Location loc = mlirModule.getLoc();
   SmallVector<const llvm::MDNode *> workList;
   SetVector<const llvm::MDNode *> nodesToConvert;
   workList.push_back(node);
   while (!workList.empty()) {
for (const llvm::Instruction &inst : llvm::instructions(func)) {
       if (aliasAnalysisNodes.NoAlias)
         if (failed(processAliasScopeMetadata(aliasAnalysisNodes.NoAlias)))
           return failure();
     }
   }
   return success();
 }
 
-LogicalResult ModuleImport::convertComdats() {
-  ComdatOp comdat = getGlobalComdatOp();
+void ModuleImport::processComdat(const llvm::Comdat *comdat) {
+  if (comdatMapping.contains(comdat))
+    return;
+
+  ComdatOp comdatOp = getGlobalComdatOp();
   OpBuilder::InsertionGuard guard(builder);
-  builder.setInsertionPointToEnd(&comdat.getBody().back());
-  for (auto &kv : llvmModule->getComdatSymbolTable()) {
-    StringRef name = kv.getKey();
-    llvm::Comdat::SelectionKind selector = kv.getValue().getSelectionKind();
-    builder.create<ComdatSelectorOp>(mlirModule.getLoc(), name,
-                                     convertComdatFromLLVM(selector));
-  }
+  builder.setInsertionPointToEnd(&comdatOp.getBody().back());
+  auto selectorOp = builder.create<ComdatSelectorOp>(
+      mlirModule.getLoc(), comdat->getName(),
+      convertComdatFromLLVM(comdat->getSelectionKind()));
+  auto symbolRef =
+      SymbolRefAttr::get(builder.getContext(), getGlobalComdatOpName(),
+                         FlatSymbolRefAttr::get(selectorOp.getSymNameAttr()));
+  comdatMapping.try_emplace(comdat, symbolRef);
 
+LogicalResult ModuleImport::convertComdats() {
+  for (llvm::GlobalVariable &globalVar : llvmModule->globals())
+    if (globalVar.hasComdat())
+      processComdat(globalVar.getComdat());
+  for (llvm::Function &func : llvmModule->functions())
+    if (func.hasComdat())
+      processComdat(func.getComdat());
   return success();
 }
 
 LogicalResult ModuleImport::convertGlobals() {
   for (llvm::GlobalVariable &globalVar : llvmModule->globals()) {
     if (globalVar.getName() == getGlobalCtorsVarName() ||
         globalVar.getName() == getGlobalDtorsVarName()) {
       if (failed(convertGlobalCtorsAndDtors(&globalVar))) {
if (globalVar->hasAtLeastLocalUnnamedAddr()) {
     globalOp.setUnnamedAddr(
         convertUnnamedAddrFromLLVM(globalVar->getUnnamedAddr()));
   }
   if (globalVar->hasSection())
     globalOp.setSection(globalVar->getSection());
   globalOp.setVisibility_(
       convertVisibilityFromLLVM(globalVar->getVisibility()));
 
-  if (globalVar->hasComdat()) {
-    llvm::Comdat *llvmComdat = globalVar->getComdat();
-    ComdatOp comdat = getGlobalComdatOp();
-    if (ComdatSelectorOp selector = dyn_cast<ComdatSelectorOp>(
-            comdat.lookupSymbol(llvmComdat->getName()))) {
-      auto symbolRef =
-          SymbolRefAttr::get(builder.getContext(), getGlobalComdatOpName(),
-                             FlatSymbolRefAttr::get(selector.getSymNameAttr()));
-      globalOp.setComdatAttr(symbolRef);
-    } else
-      return failure();
-  }
+  if (globalVar->hasComdat())
+    globalOp.setComdatAttr(comdatMapping.lookup(globalVar->getComdat()));
 
   return success();
 }
 
 LogicalResult
 ModuleImport::convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar) {
   if (!globalVar->hasInitializer() || !globalVar->hasAppendingLinkage())
     return failure();
LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   else if (func->hasPersonalityFn())
     emitWarning(funcOp.getLoc(), "could not deduce personality, skipping it");
 
   if (func->hasGC())
     funcOp.setGarbageCollector(StringRef(func->getGC()));
 
   funcOp.setVisibility_(convertVisibilityFromLLVM(func->getVisibility()));
 
+  if (func->hasComdat())
+    funcOp.setComdatAttr(comdatMapping.lookup(func->getComdat()));
+
   // Handle Function attributes.
   processFunctionAttributes(func, funcOp);
 
   // Convert non-debug metadata by using the dialect interface.
   SmallVector<std::pair<unsigned, llvm::MDNode *>> allMetadata;
   func->getAllMetadata(allMetadata);
   for (auto &[kind, node] : allMetadata) {
     if (!iface.isConvertibleMetadata(kind))
OwningOpRef<ModuleOp> module(ModuleOp::create(FileLineColLoc::get(
       StringAttr::get(context, llvmModule->getSourceFileName()), /*line=*/0,
       /*column=*/0)));
 
   ModuleImport moduleImport(module.get(), std::move(llvmModule));
   if (failed(moduleImport.initializeImportInterface()))
     return {};
   if (failed(moduleImport.convertDataLayout()))
     return {};
-  if (failed(moduleImport.convertMetadata()))
-    return {};
   if (failed(moduleImport.convertComdats()))
     return {};
+  if (failed(moduleImport.convertMetadata()))
+    return {};
   if (failed(moduleImport.convertGlobals()))
     return {};
   if (failed(moduleImport.convertFunctions()))
     return {};
 
   return module;
 }

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

for (auto op : getModuleBody(mlirModule).getOps<LLVM::GlobalOp>()) {
     auto *var = new llvm::GlobalVariable(
         *llvmModule, type, op.getConstant(), linkage, cst, op.getSymName(),
         /*InsertBefore=*/nullptr,
         op.getThreadLocal_() ? llvm::GlobalValue::GeneralDynamicTLSModel
                              : llvm::GlobalValue::NotThreadLocal,
         addrSpace);
 
     if (std::optional<mlir::SymbolRefAttr> comdat = op.getComdat()) {
-      StringRef name = comdat->getLeafReference().getValue();
-      if (!llvmModule->getComdatSymbolTable().contains(name))
-        return emitError(op.getLoc(), "global references non-existant comdat");
-      llvm::Comdat *llvmComdat = llvmModule->getOrInsertComdat(name);
-      var->setComdat(llvmComdat);
+      auto selectorOp = cast<ComdatSelectorOp>(
+          SymbolTable::lookupNearestSymbolFrom(op, *comdat));
+      var->setComdat(comdatMapping.lookup(selectorOp));
     }
 
     if (op.getUnnamedAddr().has_value())
       var->setUnnamedAddr(convertUnnamedAddrToLLVM(*op.getUnnamedAddr()));
 
     if (op.getSection().has_value())
       var->setSection(*op.getSection());
 
for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {
 
     // Forward the pass-through attributes to LLVM.
     if (failed(forwardPassthroughAttributes(
             function.getLoc(), function.getPassthrough(), llvmFunc)))
       return failure();
 
     // Convert visibility attribute.
     llvmFunc->setVisibility(convertVisibilityToLLVM(function.getVisibility_()));
+
+    // Convert the comdat attribute.
+    if (std::optional<mlir::SymbolRefAttr> comdat = function.getComdat()) {
+      auto selectorOp = cast<ComdatSelectorOp>(
+          SymbolTable::lookupNearestSymbolFrom(function, *comdat));
+      llvmFunc->setComdat(comdatMapping.lookup(selectorOp));
+    }
   }
 
   return success();
 }
 
 LogicalResult ModuleTranslation::convertFunctions() {
   // Convert functions.
   for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {
     // Ignore external functions.
     if (function.isExternal())
       continue;
 
     if (failed(convertOneFunction(function)))
       return failure();
   }
 
   return success();
 }
 
 LogicalResult ModuleTranslation::convertComdats() {
-  for (ComdatOp comdat : getModuleBody(mlirModule).getOps<ComdatOp>()) {
-    for (ComdatSelectorOp selector : comdat.getOps<ComdatSelectorOp>()) {
-      StringRef name = selector.getName();
+  for (auto comdatOp : getModuleBody(mlirModule).getOps<ComdatOp>()) {
+    for (auto selectorOp : comdatOp.getOps<ComdatSelectorOp>()) {
       llvm::Module *module = getLLVMModule();
-      if (module->getComdatSymbolTable().contains(name)) {
-        return emitError(selector.getLoc())
+      if (module->getComdatSymbolTable().contains(selectorOp.getSymName()))
+        return emitError(selectorOp.getLoc())
                << "comdat selection symbols must be unique even in different "
                   "comdat regions";
-      }
-      llvm::Comdat *comdat = module->getOrInsertComdat(name);
-      comdat->setSelectionKind(convertComdatToLLVM(selector.getComdat()));
+      llvm::Comdat *comdat = module->getOrInsertComdat(selectorOp.getSymName());
+      comdat->setSelectionKind(convertComdatToLLVM(selectorOp.getComdat()));
+      comdatMapping.try_emplace(selectorOp, comdat);
     }
   }
   return success();
 }
 
 LogicalResult ModuleTranslation::createAccessGroupMetadata() {
   return loopAnnotationTranslation->createAccessGroupMetadata();
 }
mlir::translateModuleToLLVMIR(Operation *module, llvm::LLVMContext &llvmContext,
   std::unique_ptr<llvm::Module> llvmModule =
       prepareLLVMModule(module, llvmContext, name);
   if (!llvmModule)
     return nullptr;
 
   LLVM::ensureDistinctSuccessors(module);
 
   ModuleTranslation translator(module, std::move(llvmModule));
-  if (failed(translator.convertFunctionSignatures()))
-    return nullptr;
   if (failed(translator.convertComdats()))
     return nullptr;
+  if (failed(translator.convertFunctionSignatures()))
+    return nullptr;
   if (failed(translator.convertGlobals()))
     return nullptr;
   if (failed(translator.createAccessGroupMetadata()))
     return nullptr;
   if (failed(translator.createAliasScopeMetadata()))
     return nullptr;
   if (failed(translator.createTBAAMetadata()))
     return nullptr;

mlir/test/Dialect/LLVMIR/comdat.mlir

 // RUN: mlir-opt -split-input-file -verify-diagnostics %s | FileCheck %s
 
 // CHECK: llvm.comdat @__llvm_comdat
 llvm.comdat @__llvm_comdat {
   // CHECK: llvm.comdat_selector @any_comdat any
   llvm.comdat_selector @any_comdat any
   // CHECK: llvm.comdat_selector @exactmatch_comdat exactmatch
   llvm.comdat_selector @exactmatch_comdat exactmatch
   // CHECK: llvm.comdat_selector @largest_comdat largest
   llvm.comdat_selector @largest_comdat largest
   // CHECK: llvm.comdat_selector @nodeduplicate_comdat nodeduplicate
   llvm.comdat_selector @nodeduplicate_comdat nodeduplicate
   // CHECK: llvm.comdat_selector @samesize_comdat samesize
   llvm.comdat_selector @samesize_comdat samesize
 }
-

mlir/test/Dialect/LLVMIR/func.mlir

module {
   llvm.func hidden @hidden() {
     llvm.return
   }
 
   // CHECK-LABEL: llvm.func protected @protected
   llvm.func protected @protected() {
     llvm.return
   }
+
+  // CHECK: llvm.comdat @__llvm_comdat
+  llvm.comdat @__llvm_comdat {
+    // CHECK: llvm.comdat_selector @any any
+    llvm.comdat_selector @any any
+  }
+  // CHECK: @any() comdat(@__llvm_comdat::@any) attributes
+  llvm.func @any() comdat(@__llvm_comdat::@any) attributes { dso_local } {
+    llvm.return
+  }
 }
 
 // -----
 
 module {
   // expected-error@+1 {{requires one region}}
   "llvm.func"() {function_type = !llvm.func<void ()>, sym_name = "no_region"} : () -> ()
 }

mlir/test/Dialect/LLVMIR/global.mlir

 // CHECK: llvm.mlir.global external @has_addr_space(32 : i64) {addr_space = 3 : i32} : i64
 llvm.mlir.global external @has_addr_space(32 : i64) {addr_space = 3: i32} : i64
 
 // CHECK: llvm.comdat @__llvm_comdat
 llvm.comdat @__llvm_comdat {
   // CHECK: llvm.comdat_selector @any any
   llvm.comdat_selector @any any
 }
-// CHECK: llvm.mlir.global external @any() comdat(@__llvm_comdat::@any) {addr_space = 0 : i32} : i64
-llvm.mlir.global @any() comdat(@__llvm_comdat::@any) : i64
+// CHECK: llvm.mlir.global external @any() comdat(@__llvm_comdat::@any) {addr_space = 1 : i32} : i64
+llvm.mlir.global @any() comdat(@__llvm_comdat::@any) {addr_space = 1 : i32} : i64
 
 // CHECK-LABEL: references
 func.func @references() {
   // CHECK: llvm.mlir.addressof @".string" : !llvm.ptr
   %0 = llvm.mlir.addressof @".string" : !llvm.ptr
 
   // CHECK: llvm.mlir.addressof @global : !llvm.ptr
   %1 = llvm.mlir.addressof @global : !llvm.ptr

mlir/test/Dialect/LLVMIR/invalid.mlir

 func.func @invalid_target_ext_constant() {
   // expected-error@+1 {{only zero-initializer allowed for target extension types}}
   %0 = llvm.mlir.constant(42 : index) : !llvm.target<"spirv.Event">
 }
 
 // -----
 
 llvm.comdat @__llvm_comdat {
-  // expected-error@+1 {{only comdat selector symbols can appear in a comdat region}}
+  // expected-error@below {{only comdat selector symbols can appear in a comdat region}}
   llvm.return
 }
 
 // -----
 
 llvm.mlir.global @not_comdat(0 : i32) : i32
-// expected-error@+1 {{expected comdat symbol}}
-llvm.mlir.global @invalid_comdat_use(0 : i32) comdat(@not_comdat) : i32
+// expected-error@below {{expected comdat symbol}}
+llvm.mlir.global @invalid_global_comdat(0 : i32) comdat(@not_comdat) : i32
+
+// -----
+
+// expected-error@below {{expected comdat symbol}}
+llvm.func @invalid_func_comdat() comdat(@foo) {
+  llvm.return
+}

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

-; RUN: mlir-translate -import-llvm %s | FileCheck %s
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
-; CHECK: llvm.mlir.global external @foo(42 : i64) comdat(@__llvm_global_comdat::@foo) {addr_space = 0 : i32} : i64
+; CHECK: llvm.comdat @__llvm_global_comdat {
+; CHECK: llvm.comdat_selector @foo any
+$foo = comdat any
+; CHECK: }
+
+; CHECK: llvm.mlir.global external @foo(42 : i64) comdat(@__llvm_global_comdat::@foo)
 @foo = global i64 42, comdat
-; CHECK: llvm.mlir.global external @bar(42 : i64) comdat(@__llvm_global_comdat::@foo) {addr_space = 0 : i32} : i64
+; CHECK: llvm.mlir.global external @bar(42 : i64) comdat(@__llvm_global_comdat::@foo)
 @bar = global i64 42, comdat($foo)
 
+; // -----
+
 ; CHECK: llvm.comdat @__llvm_global_comdat {
-; CHECK-DAG: llvm.comdat_selector @foo any
+; CHECK: llvm.comdat_selector @foo any
 $foo = comdat any
-; CHECK-DAG: llvm.comdat_selector @exact exactmatch
+; CHECK: }
+
+
+; CHECK: llvm.func @foo() comdat(@__llvm_global_comdat::@foo)
+define void @foo() comdat {
+  ret void
+}
+; CHECK: llvm.func @bar() comdat(@__llvm_global_comdat::@foo)
+define void @bar() comdat($foo) {
+  ret void
+}
+
+; // -----
+
+; CHECK: llvm.comdat @__llvm_global_comdat {
+; CHECK: llvm.comdat_selector @exact exactmatch
 $exact = comdat exactmatch
-; CHECK-DAG: llvm.comdat_selector @largest largest
+; CHECK: llvm.comdat_selector @largest largest
 $largest = comdat largest
-; CHECK-DAG: llvm.comdat_selector @nodedup nodeduplicate
+; CHECK: llvm.comdat_selector @nodedup nodeduplicate
 $nodedup = comdat nodeduplicate
-; CHECK-DAG: llvm.comdat_selector @same samesize
+; CHECK: llvm.comdat_selector @same samesize
 $same = comdat samesize
-
 ; CHECK: }
+
+@exact = global i64 42, comdat
+@largest = global i64 42, comdat
+@nodedup = global i64 42, comdat
+@same = global i64 42, comdat
+
+; // -----
+
+; Verify a global comdat operation is only created if there are comdats to import.
+; CHECK-NOT: llvm.comdat
+; CHECK: llvm.mlir.global external @foobar
+; CHECK-NOT: llvm.comdat
+@foobar = global i64 42

mlir/test/Target/LLVMIR/comdat.mlir

+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+llvm.comdat @__llvm_comdat {
+  // CHECK-DAG: $[[ANY:.*]] = comdat any
+  llvm.comdat_selector @any any
+  // CHECK-DAG: $[[EXACT:.*]] = comdat exactmatch
+  llvm.comdat_selector @exactmatch exactmatch
+  // CHECK-DAG: $[[LARGEST:.*]] = comdat largest
+  llvm.comdat_selector @largest largest
+  // CHECK-DAG: $[[NODEDUP:.*]] = comdat nodeduplicate
+  llvm.comdat_selector @nodeduplicate nodeduplicate
+  // CHECK-DAG: $[[SAME:.*]] = comdat samesize
+  llvm.comdat_selector @samesize samesize
+}
+
+// CHECK: @any = internal constant i64 1, comdat
+llvm.mlir.global internal constant @any(1 : i64) comdat(@__llvm_comdat::@any) : i64
+// CHECK: @any_global = internal constant i64 1, comdat($[[ANY]])
+llvm.mlir.global internal constant @any_global(1 : i64) comdat(@__llvm_comdat::@any) : i64
+// CHECK: @exact_global = internal constant i64 1, comdat($[[EXACT]])
+llvm.mlir.global internal constant @exact_global(1 : i64) comdat(@__llvm_comdat::@exactmatch) : i64
+// CHECK: @largest_global = internal constant i64 1, comdat($[[LARGEST]])
+llvm.mlir.global internal constant @largest_global(1 : i64) comdat(@__llvm_comdat::@largest) : i64
+
+// CHECK: define void @nodeduplicate() comdat
+llvm.func @nodeduplicate() comdat(@__llvm_comdat::@nodeduplicate) { llvm.return }
+// CHECK: define void @nodeduplicate_func() comdat($[[NODEDUP]])
+llvm.func @nodeduplicate_func() comdat(@__llvm_comdat::@nodeduplicate) { llvm.return }
+// CHECK: define void @samesize_func() comdat($[[SAME]])
+llvm.func @samesize_func() comdat(@__llvm_comdat::@samesize) { llvm.return }

mlir/test/Target/LLVMIR/llvmir.mlir

 // RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
 
-
-// Comdat sections
-llvm.comdat @__llvm_comdat {
-  // CHECK: $any = comdat any
-  llvm.comdat_selector @any any
-  // CHECK: $exactmatch = comdat exactmatch
-  llvm.comdat_selector @exactmatch exactmatch
-  // CHECK: $largest = comdat largest
-  llvm.comdat_selector @largest largest
-  // CHECK: $nodeduplicate = comdat nodeduplicate
-  llvm.comdat_selector @nodeduplicate nodeduplicate
-  // CHECK: $samesize = comdat samesize
-  llvm.comdat_selector @samesize samesize
-}
-
-
 // CHECK: @global_aligned32 = private global i64 42, align 32
 "llvm.mlir.global"() ({}) {sym_name = "global_aligned32", global_type = i64, value = 42 : i64, linkage = #llvm.linkage<private>, alignment = 32} : () -> ()
 
 // CHECK: @global_aligned64 = private global i64 42, align 64
 llvm.mlir.global private @global_aligned64(42 : i64) {alignment = 64 : i64} : i64
 
 // CHECK: @global_aligned64_native = private global i64 42, align 64
 llvm.mlir.global private @global_aligned64_native(42 : i64) { alignment = 64 } : i64

 //
 // Section attribute.
 //
 
 // CHECK: @sectionvar = internal constant [10 x i8] c"teststring", section ".mysection"
 llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}: !llvm.array<10 x i8>
 
 //
-// Comdat attribute.
-//
-// CHECK: @has_any_comdat = internal constant i64 1, comdat($any)
-llvm.mlir.global internal constant @has_any_comdat(1 : i64) comdat(@__llvm_comdat::@any) : i64
-// CHECK: @has_exactmatch_comdat = internal constant i64 1, comdat($exactmatch)
-llvm.mlir.global internal constant @has_exactmatch_comdat(1 : i64) comdat(@__llvm_comdat::@exactmatch) : i64
-// CHECK: @has_largest_comdat = internal constant i64 1, comdat($largest)
-llvm.mlir.global internal constant @has_largest_comdat(1 : i64) comdat(@__llvm_comdat::@largest) : i64
-// CHECK: @has_nodeduplicate_comdat = internal constant i64 1, comdat($nodeduplicate)
-llvm.mlir.global internal constant @has_nodeduplicate_comdat(1 : i64) comdat(@__llvm_comdat::@nodeduplicate) : i64
-// CHECK: @has_samesize_comdat = internal constant i64 1, comdat($samesize)
-llvm.mlir.global internal constant @has_samesize_comdat(1 : i64) comdat(@__llvm_comdat::@samesize) : i64
-
-//
 // Declarations of the allocation functions to be linked against. These are
 // inserted before other functions in the module.
 //
 
 // CHECK: declare ptr @malloc(i64)
 llvm.func @malloc(i64) -> !llvm.ptr<i8>
 // CHECK: declare void @free(ptr)