[mlir] Add loop codegen options to some LLVM dialect ops.

[arpith-jacob]

Summary

Add a Loop Option attribute and generate llvm metadata attached to
branch instructions to control code generation.

[mehdi_amini]

Thanks for the contribution :)

Why are you modeling this with a string indirection to a dictionary attribute on the module itself instead of adding the attribute directly on every individual operations?

[arpith-jacob]

Why are you modeling this with a string indirection to a dictionary attribute on the module itself instead of adding the attribute directly on every individual operations?

The *same* loop options metadata are referenced by multiple branch instructions. When translating to llvm-ir, we need to generate the loop metadata once and reference it within branch instructions. Having these loop options on a common parent-op works well in this case.

[mehdi_amini]

In D96820#2568578, @arpith-jacob wrote:

Why are you modeling this with a string indirection to a dictionary attribute on the module itself instead of adding the attribute directly on every individual operations?

The *same* loop options metadata are referenced by multiple branch instructions. When translating to llvm-ir, we need to generate the loop metadata once and reference it within branch instructions.

Why isn't falling off naturally from attribute uniquing in the context? You'd get a unique pointer comparison here.

Having these loop options on a common parent-op works well in this case.

Yes but this indirection through string tagging can become out of sync and isn't really robust from a modeling point of view, while this seems like you only introduced it as a convenience for the translation.

[mehdi_amini]

(For example: imagine linking two modules and how it would require ad-hoc process for these metadata right now)

[ftynse]

This LGTM with a bit more documentation and comments addressed.

Regarding the indirection, there are multiple operations that are associated with a loop and this modeling specifies exactly this association. The attribute on these operations is exactly a uniqued StringAttr that specifies which loops they are associated with. The loop may or may not additionally say that a loop has properties translatable to LLVM IR metadata. This is specified once per loop. Duplicating those properties in each occurrence of the attribute (that is, on each operation) will much more likely lead to divergent/incompatible properties associated with the same loop, so I agree with the current modeling. Dealing with name conflicts sounds like exactly the job of a module merger, it will already have to do that, e.g., for symbols with private visibility.

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

75–88

All these could youse some documentation.

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

215

Nit: MLIR uses /// for function- and class-level comments.

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

2170

Use .isa instead of dyn_cast if you don't need the result.

2354

Nit: we tend to prefer assert(condition && "some meaningful message") for clarity

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

627

This needs documentation.

658–659

This looks like it can fail silently, without ever reporting something to the user. The code seems to have a verifier for the attribute being an ArrayAttr, so I'd just assert here instead or had used a cast instead of dyn_cast to trigger as assertion.

663

Nit: SmallVector is reexported into mlir:: namespace so you can omit the namespace. Also, it can now automatically compute the number of stack elements based on the sizeof the element type, prefer that to an explicit number of stack elements unless there is strong reason to pick one.

684

Nit: please spell out the types unless they are too long or clear from context (e.g., there's a .cast on the RHS); here and above.

[ftynse]

mlir/test/Dialect/LLVMIR/roundtrip.mlir

424

Please also add tests for parsing failures (at least for all error messages that can be emitted in parsing)

[mehdi_amini]

In D96820#2568974, @ftynse wrote:

This LGTM with a bit more documentation and comments addressed.

Regarding the indirection, there are multiple operations that are associated with a loop and this modeling specifies exactly this association.
The attribute on these operations is exactly a uniqued StringAttr that specifies which loops they are associated with.

I'm not convinced that this attribute is intended to uniquely identify a single loop, is it?
Even if it is, then tagging a string attribute as part of the dictionary is orthogonal to the indirection, which I'm still not convinced is a good modeling right now.

The loop may or may not additionally say that a loop has properties translatable to LLVM IR metadata. This is specified once per loop. Duplicating those properties in each occurrence of the attribute (that is, on each operation) will much more likely lead to divergent/incompatible properties associated with the same loop, so I agree with the current modeling. Dealing with name conflicts sounds like exactly the job of a module merger, it will already have to do that, e.g., for symbols with private visibility.

I reiterate my concern with this modeling, I am far from convinced that this is the right things in MLIR right now, so please start an RFC on Discourse.

[mehdi_amini]

Duplicating those properties in each occurrence of the attribute (that is, on each operation) will much more likely lead to divergent/incompatible properties associated with the same loop, so I agree with the current modeling. Dealing with name conflicts sounds like exactly the job of a module merger, it will already have to do that, e.g., for symbols with private visibility.

To be more specific, I have a strongly opposed point of view here which I believe comes from how things work in LLVM as well: a module merger shouldn't have to deal with such annotation which can't be a core property of the IR (while symbols are). On the other hand a transformation that manipulate the control flow graph at the LLVM level can be required to manage metadata on branches. So the issue of "divergent/incompatible properties associated with the same loop" is intrinsic to the model, and I don't believe is solved by the indirection (creating the divergence by missing an update to an operation seems just as easy and buggy than keep the metadata in sync but with an incorrect update because you didn't process an operation).

[ftynse]

In D96820#2569049, @mehdi_amini wrote:

Even if it is, then tagging a string attribute as part of the dictionary is orthogonal to the indirection

I don't understand what you mean here. What is the indirection you are complaining about?

The loop may or may not additionally say that a loop has properties translatable to LLVM IR metadata. This is specified once per loop. Duplicating those properties in each occurrence of the attribute (that is, on each operation) will much more likely lead to divergent/incompatible properties associated with the same loop, so I agree with the current modeling. Dealing with name conflicts sounds like exactly the job of a module merger, it will already have to do that, e.g., for symbols with private visibility.

I reiterate my concern with this modeling, I am far from convinced that this is the right things in MLIR right now, so please start an RFC on Discourse.

What are the alternatives?

[mehdi_amini]

In D96820#2569063, @ftynse wrote:

In D96820#2569049, @mehdi_amini wrote:

Even if it is, then tagging a string attribute as part of the dictionary is orthogonal to the indirection

I don't understand what you mean here. What is the indirection you are complaining about?

The storage of the metadata on the module, which looks to me like a hack to address the lack of support for the LLVM "distinct metadata" in MLIR.

The loop may or may not additionally say that a loop has properties translatable to LLVM IR metadata. This is specified once per loop. Duplicating those properties in each occurrence of the attribute (that is, on each operation) will much more likely lead to divergent/incompatible properties associated with the same loop, so I agree with the current modeling. Dealing with name conflicts sounds like exactly the job of a module merger, it will already have to do that, e.g., for symbols with private visibility.

I reiterate my concern with this modeling, I am far from convinced that this is the right things in MLIR right now, so please start an RFC on Discourse.

What are the alternatives?

I believe we're hitting here the fact that LLVM models this with distinct metadata, which isn't a concept in MLIR. So either we should model things differently in MLIR for concepts that are using a "distinct metadata" or we should provide an equivalent concept to allow this modeling (and I'm not convinced that a DictionnaryAttribute on the module is the best modeling, but we can evaluate this in a Discourse thread, I'm interested to discuss this further with River as well).
This is something that will come up every time we need to handle this kind of metadata, and is worth a solid design.

[arpith-jacob]

Addressed comments.

[arpith-jacob]

Updated implementation to use an llvm.metadata operation as suggested in
https://llvm.discourse.group/t/annotating-loops-with-codegen-options/2854

[ftynse]

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

861

Nit: we usually add the keyword attributes before attr-dict if there is also a region present.

mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp

245

This function never fails, so let's make it return void

[arpith-jacob]

Thanks for the review. Addressed all comments.

[mehdi_amini]

Great, thanks!

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

2205

Nit: be mindful of auto in general, keep it to where it makes the code more readable and/or the type is obvious from the context.

mlir/test/Target/llvmir.mlir

1483

The syntax is quite heavy here, can we make it look something like: options = #llvm.loopopt<disable_unroll = true, disable_licm = true, interleave_count = 1> ?

[arpith-jacob]

Replaced auto with type in some instances.

[arpith-jacob]

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

2205

Added the specific type in several places to improve readability.

mlir/test/Target/llvmir.mlir

1483

Each option is a distinct attribute. It seems to me that the mlir infrastructure requires prefixing "llvm." to each llvm dialect specific attribute, thus the "llvm.loopopt" prefix. I'll leave it as is.

[mehdi_amini]

mlir/test/Target/llvmir.mlir

1483

I'm sure River has a suggestion here :)

[mehdi_amini]

mlir/test/Target/llvmir.mlir

1483

Seems like the most convenient is D97589, but I'm fine landing this patch and migrating it to D97589 afterward!

[arpith-jacob]

mlir/test/Target/llvmir.mlir

1483

SG. I'll investigate D97589 once it lands.

Could you please commit this revision? Thank you.

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

 namespace mlir {
 namespace LLVM {
 class LLVMDialect;
 
 namespace detail {
 struct LLVMTypeStorage;
 struct LLVMDialectImpl;
 struct BitmaskEnumStorage;
+struct LoopOptionAttrStorage;
 } // namespace detail
 
 /// An attribute that specifies LLVM instruction fastmath flags.
 class FMFAttr : public Attribute::AttrBase<FMFAttr, Attribute,
                                            detail::BitmaskEnumStorage> {
 public:
   using Base::Base;
 
   static FMFAttr get(FastmathFlags flags, MLIRContext *context);
 
   FastmathFlags getFlags() const;
 
   void print(DialectAsmPrinter &p) const;
   static Attribute parse(DialectAsmParser &parser);
 };
 
+/// An attribute that specifies LLVM loop codegen options.
+class LoopOptionAttr
+    : public Attribute::AttrBase<LoopOptionAttr, Attribute,
+                                 detail::LoopOptionAttrStorage> {
+public:
+  using Base::Base;
+
+  static LoopOptionAttr getParallelAccess(MLIRContext *context,
+                                          bool parallel = true);
+  static LoopOptionAttr getDisableUnroll(MLIRContext *context,
+                                         bool disable = true);
+  static LoopOptionAttr getDisableLICM(MLIRContext *context,
+                                       bool disable = true);
+  static LoopOptionAttr getInterleaveCount(MLIRContext *context, int32_t count);
+
+  LoopOptionCase getCase() const;
+  bool getBool() const;
+  int32_t getInt() const;
+
+  void print(DialectAsmPrinter &p) const;
+  static Attribute parse(DialectAsmParser &parser);
+};
+
 } // namespace LLVM
 } // namespace mlir
 
 ///// Ops /////
 #define GET_OP_CLASSES
 #include "mlir/Dialect/LLVMIR/LLVMOps.h.inc"
 
 #include "mlir/Dialect/LLVMIR/LLVMOpsDialect.h.inc"

mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td

def LLVM_Dialect : Dialect {
 
   let hasRegionArgAttrVerify = 1;
   let hasOperationAttrVerify = 1;
   let extraClassDeclaration = [{
     /// Name of the data layout attributes.
     static StringRef getDataLayoutAttrName() { return "llvm.data_layout"; }
     static StringRef getAlignAttrName() { return "llvm.align"; }
     static StringRef getNoAliasAttrName() { return "llvm.noalias"; }
+    static StringRef getLoopOptionsAttrName() { return "llvm.loops"; }
+    static StringRef getLoopAttrName() { return "llvm.loop"; }
 
     /// Verifies if the given string is a well-formed data layout descriptor.
     /// Uses `reportError` to report errors.
     static LogicalResult verifyDataLayoutString(
         StringRef descr, llvm::function_ref<void (const Twine &)> reportError);
 
     /// Name of the target triple attribute.
     static StringRef getTargetTripleAttrName() { return "llvm.target_triple"; }

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

def LLVM_FMFAttr : DialectAttr<
     "LLVM fastmath flags"> {
   let storageType = "::mlir::LLVM::FMFAttr";
   let returnType = "::mlir::LLVM::FastmathFlags";
   let convertFromStorage = "$_self.getFlags()";
   let constBuilderCall =
           "::mlir::LLVM::FMFAttr::get($0, $_builder.getContext())";
 }
 
+def LOptParallelAccess : I32EnumAttrCase<"parallel_access", 1>;
+def LOptDisableUnroll : I32EnumAttrCase<"disable_unroll", 2>;
+def LOptDisableLICM : I32EnumAttrCase<"disable_licm", 3>;
+def LOptInterleaveCount : I32EnumAttrCase<"interleave_count", 4>;
+
+def LoopOptionCase : I32EnumAttr<
+    "LoopOptionCase",
+    "LLVM loop option",
+    [LOptParallelAccess, LOptDisableUnroll, LOptDisableLICM, LOptInterleaveCount
+    ]> {
+  let cppNamespace = "::mlir::LLVM";
+}
+
 class LLVM_Builder<string builder> {
   string llvmBuilder = builder;
 }
 
 def LLVM_OneResultOpBuilder :
   OpBuilderDAG<(ins "Type":$resultType, "ValueRange":$operands,
     CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes),
   [{
def LLVM_GlobalOp : LLVM_Op<"mlir.global",
   let description = [{
     Since MLIR allows for arbitrary operations to be present at the top level,
     global variables are defined using the `llvm.mlir.global` operation. Both
     global constants and variables can be defined, and the value may also be
     initialized in both cases.
 
     There are two forms of initialization syntax. Simple constants that can be
     represented as MLIR attributes can be given in-line:
 
     ```mlir
     llvm.mlir.global @variable(32.0 : f32) : f32
     ```
 
     This initialization and type syntax is similar to `llvm.mlir.constant` and
     may use two types: one for MLIR attribute and another for the LLVM value.
     These types must be compatible.
 

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

translateModule(Operation *m, llvm::LLVMContext &llvmContext,
 
     LLVM::ensureDistinctSuccessors(m);
 
     T translator(m, std::move(llvmModule));
     if (failed(translator.convertFunctionSignatures()))
       return nullptr;
     if (failed(translator.convertGlobals()))
       return nullptr;
+    if (failed(translator.createLoopMetadata()))
+      return nullptr;
     if (failed(translator.convertFunctions()))
       return nullptr;
 
     return std::move(translator.llvmModule);
   }
 
   /// A helper method to get the single Block in an operation honoring LLVM's
   /// module requirements.
public:
   }
 
   /// Finds an LLVM IR instruction that corresponds to the given MLIR operation
   /// with successors.
   llvm::Instruction *lookupBranch(Operation *op) const {
     return branchMapping.lookup(op);
   }
 
+  /// Finds the LLVM metadata corresponding to a loop..
+  llvm::MDNode *lookupLoopMetadata(StringRef loop) const {
+    return loopMetadataMapping.lookup(loop);
+  }
+
   /// Converts the type from MLIR LLVM dialect to LLVM.
   llvm::Type *convertType(Type type);
 
   /// Looks up remapped a list of remapped values.
   SmallVector<llvm::Value *, 8> lookupValues(ValueRange values);
 
   /// Returns the MLIR context of the module being translated.
   MLIRContext &getContext() { return *mlirModule->getContext(); }
private:
   /// Check whether the module contains only supported ops directly in its body.
   static LogicalResult checkSupportedModuleOps(Operation *m);
 
   LogicalResult convertFunctionSignatures();
   LogicalResult convertFunctions();
   LogicalResult convertGlobals();
   LogicalResult convertOneFunction(LLVMFuncOp func);
 
+  // Creates LLVM metadata for loops annotated in this module if the module
+  // contains an `llvm.loops` attribute. Each element of this attribute is a
+  // NamedAttribute and describes the attributes for a distinct loop. The first
+  // element of the NamedAttribute is a string attribute identifying the loop
+  // and the second is an array attribute holding the loop options.
+  LogicalResult createLoopMetadata();
+
   /// Translates dialect attributes attached to the given operation.
   LogicalResult convertDialectAttributes(Operation *op);
 
   /// Original and translated module.
   Operation *mlirModule;
   std::unique_ptr<llvm::Module> llvmModule;
   /// A converter for translating debug information.
   std::unique_ptr<detail::DebugTranslation> debugTranslation;
private:
   llvm::StringMap<llvm::Function *> functionMapping;
   DenseMap<Value, llvm::Value *> valueMapping;
   DenseMap<Block *, llvm::BasicBlock *> blockMapping;
 
   /// A mapping between MLIR LLVM dialect terminators and LLVM IR terminators
   /// they are converted to. This allows for connecting PHI nodes to the source
   /// values after all operations are converted.
   DenseMap<Operation *, llvm::Instruction *> branchMapping;
+
+  // Mapping from a named loop to its LLVM metadata. This map is
+  // populated on module entry and used to annotate loops (as
+  // identified via their branches) and contained memory accesses.
+  llvm::StringMap<llvm::MDNode *> loopMetadataMapping;
 };
 
 namespace detail {
 /// For all blocks in the region that were converted to LLVM IR using the given
 /// ModuleTranslation, connect the PHI nodes of the corresponding LLVM IR blocks
 /// to the results of preceding blocks.
 void connectPHINodes(Region &region, const ModuleTranslation &state);
 

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

struct BitmaskEnumStorage : public AttributeStorage {
   static BitmaskEnumStorage *construct(AttributeStorageAllocator &allocator,
                                        const KeyTy &key) {
     return new (allocator.allocate<BitmaskEnumStorage>())
         BitmaskEnumStorage(key);
   }
 
   KeyTy value = 0;
 };
+
+struct LoopOptionAttrStorage : public AttributeStorage {
+  using KeyTy = std::pair<uint64_t, int32_t>;
+
+  explicit LoopOptionAttrStorage(uint64_t option, int32_t value)
+      : option(option), value(value) {}
+
+  bool operator==(const KeyTy &key) const {
+    return key == KeyTy(option, value);
+  }
+
+  static LoopOptionAttrStorage *
+  construct(mlir::AttributeStorageAllocator &allocator, const KeyTy &key) {
+    return new (allocator.allocate<LoopOptionAttrStorage>())
+        LoopOptionAttrStorage(key.first, key.second);
+  }
+
+  uint64_t option;
+  int32_t value;
+};
 } // namespace detail
 } // namespace LLVM
 } // namespace mlir
 
 static auto processFMFAttr(ArrayRef<NamedAttribute> attrs) {
   SmallVector<NamedAttribute, 8> filteredAttrs(
       llvm::make_filter_range(attrs, [&](NamedAttribute attr) {
         if (attr.first == "fastmathFlags") {
static LogicalResult verify(FenceOp &op) {
   return success();
 }
 
 //===----------------------------------------------------------------------===//
 // LLVMDialect initialization, type parsing, and registration.
 //===----------------------------------------------------------------------===//
 
 void LLVMDialect::initialize() {
-  addAttributes<FMFAttr>();
+  addAttributes<FMFAttr, LoopOptionAttr>();
 
   // clang-format off
   addTypes<LLVMVoidType,
            LLVMPPCFP128Type,
            LLVMX86MMXType,
            LLVMTokenType,
            LLVMLabelType,
            LLVMMetadataType,
LogicalResult LLVMDialect::verifyDataLayoutString(
   llvm::logAllUnhandledErrors(maybeDataLayout.takeError(), messageStream);
   reportError("invalid data layout descriptor: " + messageStream.str());
   return failure();
 }
 
 /// Verify LLVM dialect attributes.
 LogicalResult LLVMDialect::verifyOperationAttribute(Operation *op,
                                                     NamedAttribute attr) {
+  if (attr.first.strref() == getLoopOptionsAttrName()) {
+    auto dictAttr = attr.second.dyn_cast<DictionaryAttr>();
+    if (!dictAttr)
+      return op->emitOpError() << "expected '" << getLoopOptionsAttrName()
+                               << "' to be a dictionary attribute";
+    for (auto keyValue : dictAttr) {
+      auto loopOptions = keyValue.second.dyn_cast<ArrayAttr>();
+      if (!loopOptions)
+        return op->emitOpError()
+               << "expected loop options for loop '" << keyValue.first
+               << "' in attribute '" << getLoopOptionsAttrName()
+               << "' to be an array attribute";
+      for (auto loopOption : loopOptions) {
+        if (!loopOption.dyn_cast<LoopOptionAttr>())
+          return op->emitOpError()
+                 << "expected loop option for loop '" << keyValue.first
+                 << "' in attribute '" << getLoopOptionsAttrName()
+                 << "' to be a loop-option attribute";
+      }
+    }
+    return success();
+  }
+
   // If the data layout attribute is present, it must use the LLVM data layout
   // syntax. Try parsing it and report errors in case of failure. Users of this
   // attribute may assume it is well-formed and can pass it to the (asserting)
   // llvm::DataLayout constructor.
   if (attr.first.strref() != LLVM::LLVMDialect::getDataLayoutAttrName())
     return success();
   if (auto stringAttr = attr.second.dyn_cast<StringAttr>())
     return verifyDataLayoutString(

 LogicalResult LLVMDialect::verifyRegionArgAttribute(Operation *op,
                                                     unsigned regionIdx,
                                                     unsigned argIdx,
                                                     NamedAttribute argAttr) {
   // Check that llvm.noalias is a boolean attribute.
   if (argAttr.first == LLVMDialect::getNoAliasAttrName() &&
       !argAttr.second.isa<BoolAttr>())
     return op->emitError()
            << "llvm.noalias argument attribute of non boolean type";
   // Check that llvm.align is an integer attribute.
   if (argAttr.first == LLVMDialect::getAlignAttrName() &&
       !argAttr.second.isa<IntegerAttr>())
     return op->emitError()
            << "llvm.align argument attribute of non integer type";
   return success();
 }
 
if (failed(parser.parseOptionalGreater())) {
 
     if (failed(parser.parseGreater()))
       return {};
   }
 
   return FMFAttr::get(flags, parser.getBuilder().getContext());
 }
 
+LoopOptionAttr LoopOptionAttr::getParallelAccess(MLIRContext *context,
+                                                 bool parallel) {
+  auto option = LoopOptionCase::parallel_access;
+  return Base::get(context, static_cast<uint64_t>(option),
+                   static_cast<int32_t>(parallel));
+}
+
+LoopOptionAttr LoopOptionAttr::getDisableUnroll(MLIRContext *context,
+                                                bool disable) {
+  auto option = LoopOptionCase::disable_unroll;
+  return Base::get(context, static_cast<uint64_t>(option),
+                   static_cast<int32_t>(disable));
+}
+
+LoopOptionAttr LoopOptionAttr::getDisableLICM(MLIRContext *context,
+                                              bool disable) {
+  auto option = LoopOptionCase::disable_licm;
+  return Base::get(context, static_cast<uint64_t>(option),
+                   static_cast<int32_t>(disable));
+}
+
+LoopOptionAttr LoopOptionAttr::getInterleaveCount(MLIRContext *context,
+                                                  int32_t count) {
+  auto option = LoopOptionCase::interleave_count;
+  return Base::get(context, static_cast<uint64_t>(option),
+                   static_cast<int32_t>(count));
+}
+
+LoopOptionCase LoopOptionAttr::getCase() const {
+  return static_cast<LoopOptionCase>(getImpl()->option);
+}
+
+bool LoopOptionAttr::getBool() const {
+  LoopOptionCase option = getCase();
+  (void)option;
+  assert(option == LoopOptionCase::parallel_access ||
+         option == LoopOptionCase::disable_licm ||
+         option == LoopOptionCase::disable_unroll);
+  return static_cast<bool>(getImpl()->value);
+}
+
+int32_t LoopOptionAttr::getInt() const {
+  LoopOptionCase option = getCase();
+  (void)option;
+  assert(option == LoopOptionCase::interleave_count);
+  return getImpl()->value;
+}
+
+void LoopOptionAttr::print(DialectAsmPrinter &printer) const {
+  printer << "loopopt<" << stringifyEnum(getCase()) << " = ";
+  switch (getCase()) {
+  case LoopOptionCase::parallel_access:
+  case LoopOptionCase::disable_licm:
+  case LoopOptionCase::disable_unroll:
+    printer << (getBool() ? "true" : "false");
+    break;
+  case LoopOptionCase::interleave_count:
+    printer << getInt();
+    break;
+  }
+  printer << ">";
+}
+
+Attribute LoopOptionAttr::parse(DialectAsmParser &parser) {
+  if (failed(parser.parseLess()))
+    return {};
+
+  StringRef optionName;
+  if (failed(parser.parseKeyword(&optionName)))
+    return {};
+
+  auto option = symbolizeLoopOptionCase(optionName);
+  if (!option) {
+    parser.emitError(parser.getNameLoc(), "Unknown loop option: ")
+        << optionName;
+    return {};
+  }
+
+  if (failed(parser.parseEqual()))
+    return {};
+
+  int32_t value;
+  switch (*option) {
+  case LoopOptionCase::parallel_access:
+  case LoopOptionCase::disable_licm:
+  case LoopOptionCase::disable_unroll:
+    if (succeeded(parser.parseOptionalKeyword("true")))
+      value = 1;
+    else if (succeeded(parser.parseOptionalKeyword("false")))
+      value = 0;
+    else
+      return {};
+    break;
+  case LoopOptionCase::interleave_count:
+    if (failed(parser.parseInteger(value)))
+      return {};
+    break;
+  }
+
+  if (failed(parser.parseGreater()))
+    return {};
+
+  return Base::get(parser.getBuilder().getContext(),
+                   static_cast<uint64_t>(*option), value);
+}
+
 Attribute LLVMDialect::parseAttribute(DialectAsmParser &parser,
                                       Type type) const {
   if (type) {
     parser.emitError(parser.getNameLoc(), "unexpected type");
     return {};
   }
   StringRef attrKind;
   if (parser.parseKeyword(&attrKind))
     return {};
 
   if (attrKind == "fastmath")
     return FMFAttr::parse(parser);
 
-  parser.emitError(parser.getNameLoc(), "Unknown attrribute type: ")
-      << attrKind;
+  if (attrKind == "loopopt")
+    return LoopOptionAttr::parse(parser);
+
+  parser.emitError(parser.getNameLoc(), "Unknown attribute type: ") << attrKind;
   return {};
 }
 
 void LLVMDialect::printAttribute(Attribute attr, DialectAsmPrinter &os) const {
   if (auto fmf = attr.dyn_cast<FMFAttr>())
     fmf.print(os);
+  else if (auto lopt = attr.dyn_cast<LoopOptionAttr>())
+    lopt.print(os);
   else
     llvm_unreachable("Unknown attribute type");
 }

mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp

static llvm::FastMathFlags getFastmathFlags(FastmathFlagsInterface &op) {
   auto fmf = op.fastmathFlags();
   for (auto it : handlers)
     if (bitEnumContains(fmf, it.first))
       (ret.*(it.second))(true);
   return ret;
 }
 
 static LogicalResult
+setLoopMetadata(Operation &opInst, llvm::Instruction &llvmInst,
+                llvm::IRBuilderBase &builder,
+                LLVM::ModuleTranslation &moduleTranslation) {
+  if (Attribute attr = opInst.getAttr(LLVMDialect::getLoopAttrName())) {
+    llvm::Module *module = builder.GetInsertBlock()->getModule();
+    auto loopName = attr.cast<StringAttr>().getValue();
+    llvm::MDNode *loopMD = moduleTranslation.lookupLoopMetadata(loopName);
+    if (!loopMD)
+      return failure();
+    llvmInst.setMetadata(module->getMDKindID("llvm.loop"), loopMD);
+  }
+  return success();
+}
+
+static LogicalResult
 convertOperationImpl(Operation &opInst, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
   auto extractPosition = [](ArrayAttr attr) {
     SmallVector<unsigned, 4> position;
     position.reserve(attr.size());
     for (Attribute v : attr)
       position.push_back(v.cast<IntegerAttr>().getValue().getZExtValue());
     return position;
if (auto inlineAsmOp = dyn_cast<LLVM::InlineAsmOp>(opInst)) {
     operandTypes.reserve(inlineAsmOp.operands().size());
     for (auto t : inlineAsmOp.operands().getTypes())
       operandTypes.push_back(t);
 
     Type resultType;
     if (inlineAsmOp.getNumResults() == 0) {
       resultType = LLVM::LLVMVoidType::get(&moduleTranslation.getContext());
     } else {
       assert(inlineAsmOp.getNumResults() == 1);
       resultType = inlineAsmOp.getResultTypes()[0];
     }
     auto ft = LLVM::LLVMFunctionType::get(resultType, operandTypes);
     llvm::InlineAsm *inlineAsmInst =
         inlineAsmOp.asm_dialect().hasValue()
             ? llvm::InlineAsm::get(
                   static_cast<llvm::FunctionType *>(
                       moduleTranslation.convertType(ft)),
#include "mlir/Dialect/LLVMIR/LLVMConversions.inc"
   }
 
   // Emit branches.  We need to look up the remapped blocks and ignore the block
   // arguments that were transformed into PHI nodes.
   if (auto brOp = dyn_cast<LLVM::BrOp>(opInst)) {
     llvm::BranchInst *branch =
         builder.CreateBr(moduleTranslation.lookupBlock(brOp.getSuccessor()));
     moduleTranslation.mapBranch(&opInst, branch);
-    return success();
+    return setLoopMetadata(opInst, *branch, builder, moduleTranslation);
   }
   if (auto condbrOp = dyn_cast<LLVM::CondBrOp>(opInst)) {
     auto weights = condbrOp.branch_weights();
     llvm::MDNode *branchWeights = nullptr;
     if (weights) {
       // Map weight attributes to LLVM metadata.
       auto trueWeight =
           weights.getValue().getValue(0).cast<IntegerAttr>().getInt();
       auto falseWeight =
           weights.getValue().getValue(1).cast<IntegerAttr>().getInt();
       branchWeights =
           llvm::MDBuilder(moduleTranslation.getLLVMContext())
               .createBranchWeights(static_cast<uint32_t>(trueWeight),
                                    static_cast<uint32_t>(falseWeight));
     }
     llvm::BranchInst *branch = builder.CreateCondBr(
         moduleTranslation.lookupValue(condbrOp.getOperand(0)),
         moduleTranslation.lookupBlock(condbrOp.getSuccessor(0)),
         moduleTranslation.lookupBlock(condbrOp.getSuccessor(1)), branchWeights);
     moduleTranslation.mapBranch(&opInst, branch);
-    return success();
+    return setLoopMetadata(opInst, *branch, builder, moduleTranslation);
   }
   if (auto switchOp = dyn_cast<LLVM::SwitchOp>(opInst)) {
     llvm::MDNode *branchWeights = nullptr;
     if (auto weights = switchOp.branch_weights()) {
       llvm::SmallVector<uint32_t> weightValues;
       weightValues.reserve(weights->size());
       for (llvm::APInt weight : weights->cast<DenseIntElementsAttr>())
         weightValues.push_back(weight.getLimitedValue());

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {
 
     if (failed(convertOneFunction(function)))
       return failure();
   }
 
   return success();
 }
 
+static llvm::MDNode *getLoopOptionMetadata(llvm::LLVMContext &ctx,
+                                           LoopOptionAttr option) {
+  StringRef name;
+  llvm::Constant *value = nullptr;
+  switch (option.getCase()) {
+  case LoopOptionCase::parallel_access:
+    llvm_unreachable("incorrectly handled loop option: parallel_access");
+  case LoopOptionCase::disable_licm:
+    name = "llvm.licm.disable";
+    value = llvm::ConstantInt::getBool(ctx, option.getBool());
+    break;
+  case LoopOptionCase::disable_unroll:
+    name = "llvm.loop.unroll.disable";
+    value = llvm::ConstantInt::getBool(ctx, option.getBool());
+    break;
+  case LoopOptionCase::interleave_count:
+    name = "llvm.loop.interleave.count";
+    value = llvm::ConstantInt::get(llvm::IntegerType::get(ctx, /*NumBits=*/32),
+                                   option.getInt());
+    break;
+  }
+  return llvm::MDNode::get(ctx, {llvm::MDString::get(ctx, name),
+                                 llvm::ConstantAsMetadata::get(value)});
+}
+
+LogicalResult ModuleTranslation::createLoopMetadata() {
+  if (auto loopOptions = mlirModule->getAttrOfType<mlir::DictionaryAttr>(
+          LLVMDialect::getLoopOptionsAttrName())) {
+    for (auto nameAndMetadata : loopOptions) {
+      StringRef loopName = nameAndMetadata.first.strref();
+      auto loopMetadataAttr = nameAndMetadata.second.dyn_cast<ArrayAttr>();
+      if (!loopMetadataAttr)
+        return failure();
+
+      llvm::IRBuilder<> builder(llvmModule->getContext());
+      llvm::LLVMContext &ctx = llvmModule->getContext();
+      llvm::SmallVector<llvm::Metadata *, 8> loopOptions;
+
+      // Reserve operand 0 for loop id self reference.
+      auto dummy = llvm::MDNode::getTemporary(ctx, llvm::None);
+      loopOptions.push_back(dummy.get());
+
+      // Handle the parallel-access option as a special case.
+      auto loopOptionsAttr = loopMetadataAttr.getAsRange<LoopOptionAttr>();
+      bool isParallelAccess =
+          llvm::any_of(loopOptionsAttr, [](LoopOptionAttr option) {
+            return option.getCase() == LoopOptionCase::parallel_access &&
+                   option.getBool();
+          });
+      if (isParallelAccess) {
+        llvm::MDNode *accessGroup = llvm::MDNode::getDistinct(ctx, {});
+        llvm::MDNode *parallelAccess = llvm::MDNode::get(
+            ctx, {llvm::MDString::get(ctx, "llvm.loop.parallel_accesses"),
+                  accessGroup});
+        loopOptions.push_back(parallelAccess);
+      }
+
+      for (auto loopOption : loopOptionsAttr)
+        if (loopOption.getCase() != LoopOptionCase::parallel_access)
+          loopOptions.push_back(getLoopOptionMetadata(ctx, loopOption));
+
+      // Create loop options and set the first operand to itself.
+      llvm::MDNode *metadata = llvm::MDNode::get(ctx, loopOptions);
+      metadata->replaceOperandWith(0, metadata);
+
+      // Store the metadata in a map for later reference. These will be attached
+      // to the loop.
+      loopMetadataMapping.insert({loopName, metadata});
+    }
+  }
+
+  return success();
+}
+
 llvm::Type *ModuleTranslation::convertType(Type type) {
   return typeTranslator.translateType(type);
 }
 
 /// A helper to look up remapped operands in the value remapping table.`
 SmallVector<llvm::Value *, 8>
 ModuleTranslation::lookupValues(ValueRange values) {
   SmallVector<llvm::Value *, 8> remapped;

mlir/test/Dialect/LLVMIR/roundtrip.mlir

// CHECK: {{.*}} = llvm.fadd %arg0, %arg1 : f32
   %8 = llvm.fadd %arg0, %arg1 {fastmathFlags = #llvm.fastmath<>} : f32
 // CHECK: {{.*}} = llvm.fadd %arg0, %arg1 {fastmathFlags = #llvm.fastmath<nnan, ninf>} : f32
   %9 = llvm.fadd %arg0, %arg1 {fastmathFlags = #llvm.fastmath<nnan, ninf>} : f32
 
 // CHECK: {{.*}} = llvm.fneg %arg0 : f32
   %10 = llvm.fneg %arg0 {fastmathFlags = #llvm.fastmath<>} : f32
   return
 }
+
+// CHECK: module attributes {llvm.loops = {loop1 = [#llvm.loopopt<parallel_access = true>, #llvm.loopopt<disable_unroll = true>, #llvm.loopopt<disable_licm = true>, #llvm.loopopt<interleave_count = 1>]}}
+module attributes {llvm.loops = {loop1 = [#llvm.loopopt<parallel_access = true>, #llvm.loopopt<disable_unroll = true>, #llvm.loopopt<disable_licm = true>, #llvm.loopopt<interleave_count = 1>]}} {
+}

mlir/test/Target/llvmir-invalid.mlir

 
 // expected-error @+1 {{expected 'passthrough' to contain string or array attributes}}
 llvm.func @passthrough_wrong_type() attributes {passthrough = [42]}
 
 // -----
 
 // expected-error @+1 {{expected arrays within 'passthrough' to contain two strings}}
 llvm.func @passthrough_wrong_type() attributes {passthrough = [[42, 42]]}
+
+// -----
+
+// expected-error @+1 {{expected 'llvm.loops' to be a dictionary attribute}}
+module attributes {llvm.loops = []} {
+}
+
+// -----
+
+// expected-error @+1 {{expected loop options for loop 'loop1' in attribute 'llvm.loops' to be an array attribute}}
+module attributes {llvm.loops = {loop1 = "test"}} {
+}
+
+// -----
+
+// expected-error @+1 {{expected loop option for loop 'loop1' in attribute 'llvm.loops' to be a loop-option attribute}}
+module attributes {llvm.loops = {loop1 = ["test"]}} {
+}

mlir/test/Target/llvmir.mlir

 ^bb2(%4: i32, %5: i32): // pred: ^bb0
   llvm.return %5 : i32
 
 ^bb3: // pred: ^bb0
   llvm.return %arg0 : i32
 }
 
 // CHECK: ![[SWITCH_WEIGHT_NODE]] = !{!"branch_weights", i32 13, i32 17, i32 19}
+
+// -----
+
+module attributes {llvm.loops = {loop1 = [#llvm.loopopt<parallel_access = true>, #llvm.loopopt<disable_unroll = true>, #llvm.loopopt<disable_licm = true>, #llvm.loopopt<interleave_count = 1>]}} {
+  llvm.func @loopOptions(%arg1 : i32, %arg2 : i32) {
+      %0 = llvm.mlir.constant(0 : i32) : i32
+      llvm.br ^bb3(%0 : i32)
+    ^bb3(%1: i32):
+      %2 = llvm.icmp "slt" %1, %arg1 : i32
+      // CHECK: br i1 {{.*}} !llvm.loop ![[LOOP_NODE:[0-9]+]]
+      llvm.cond_br %2, ^bb4, ^bb5 {llvm.loop = "loop1"}
+    ^bb4:
+      %3 = llvm.add %1, %arg2  : i32
+      // CHECK: br label {{.*}} !llvm.loop ![[LOOP_NODE]]
+      llvm.br ^bb3(%3 : i32) {llvm.loop = "loop1"}
+    ^bb5:
+      llvm.return
+  }
+}
+
+// CHECK: ![[LOOP_NODE]] = distinct !{![[LOOP_NODE]], ![[PA_NODE:[0-9]+]], ![[UNROLL_DISABLE_NODE:[0-9]+]], ![[LICM_DISABLE_NODE:[0-9]+]], ![[INTERLEAVE_NODE:[0-9]+]]}
+// CHECK: ![[PA_NODE]] = !{!"llvm.loop.parallel_accesses", ![[GROUP_NODE:[0-9]+]]}
+// CHECK: ![[GROUP_NODE]] = distinct !{}
+// CHECK: ![[UNROLL_DISABLE_NODE]] = !{!"llvm.loop.unroll.disable", i1 true}
+// CHECK: ![[LICM_DISABLE_NODE]] = !{!"llvm.licm.disable", i1 true}
+// CHECK: ![[INTERLEAVE_NODE]] = !{!"llvm.loop.interleave.count", i32 1}