[mlir][LLVM] Replace readnone with memory effects

[Dinistro]

Summary

This commit introduces LLVM's MemoryEffects attribute and replaces the
deprecated usage of llvm.readnone in the LLVM dialect.
The absence of the attribute on a LLVMFuncOp implies that it might
access all kinds of memory. This semantic corresponds to llvm::Function's
behaviour.

Depends on D142002

[gysit]

mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td

386

nit: I think most other attributes use camelCase for LLVM attribute names infavor of using them as is.

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

390

nit: I would compute the attribute here and then use setMemoryAttr on the newFuncOp further below. That way we do not use the string to identify the right attribute.

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

914

It does not seem like the function ever return failure?

mlir/test/Target/LLVMIR/function-attributes.mlir

1–18 ↗

(On Diff #490129)

is this file accidentally here? Generally it may be a good idea to factor out a separate test for the function attributes? However this seems to be a copy?

[Dinistro]

addressing review comments

[gysit]

LGTM!

mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td

let builders = [
     TypeBuilder<(ins "ArrayRef<DITypeAttr>":$types), [{
       return $_get($_ctxt, /*callingConvention=*/0, types);
     }]>
   ];
   let assemblyFormat = "`<` struct(params) `>`";
   let genVerifyDecl = 1;
 }
 
+//===----------------------------------------------------------------------===//
+// MemoryEffectsAttr
+//===----------------------------------------------------------------------===//
+
+def LLVM_MemoryEffectsAttr : LLVM_Attr<"MemoryEffects", "memory_effects", [
+    SubElementAttrInterface
+  ]> {
+  let parameters = (ins
+    "ModRefInfo":$other,
+    "ModRefInfo":$argMem,
+    "ModRefInfo":$inaccessibleMem
+  );
+  let extraClassDeclaration = [{
+    bool isReadWrite();
+  }];
+  let builders = [
+    TypeBuilder<(ins "ArrayRef<ModRefInfo>":$memInfoArgs)>
+  ];
+  let assemblyFormat = "`<` struct(params) `>`";
+}
+
 #endif // LLVMIR_ATTRDEFS

mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td

 def UnnamedAddr : LLVM_EnumAttr<
     "UnnamedAddr",
     "::llvm::GlobalValue::UnnamedAddr",
     "LLVM GlobalValue UnnamedAddr",
     [UnnamedAddrNone, UnnamedAddrLocal, UnnamedAddrGlobal]> {
   let cppNamespace = "::mlir::LLVM";
 }
 
+//===----------------------------------------------------------------------===//
+// ModRefInfo
+//===----------------------------------------------------------------------===//
+
+def ModRefInfoNoModRef : LLVM_EnumAttrCase<"NoModRef", "none", "NoModRef", 0>;
+def ModRefInfoRef : LLVM_EnumAttrCase<"Ref", "read", "Ref", 1>;
+def ModRefInfoMod : LLVM_EnumAttrCase<"Mod", "write", "Mod", 2>;
+def ModRefInfoModRef : LLVM_EnumAttrCase<"ModRef", "readwrite", "ModRef", 3>;
+
+def ModRefInfoEnum : LLVM_EnumAttr<
+    "ModRefInfo",
+    "::llvm::ModRefInfo",
+    "LLVM ModRefInfo",
+    [ModRefInfoNoModRef, ModRefInfoRef, ModRefInfoMod, ModRefInfoModRef]> {
+  let cppNamespace = "::mlir::LLVM";
+}
+
 #endif // LLVMIR_ENUMS_TD

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

let extraClassDeclaration = [{
     /// Name of the C wrapper emission attribute.
     static StringRef getEmitCWrapperAttrName() {
       return "llvm.emit_c_interface";
     }
 
     /// Returns `true` if the given type is compatible with the LLVM dialect.
     static bool isCompatibleType(Type);
 
-    /// Name of the attribute mapped to LLVM's 'readnone' function attribute.
-    /// It is allowed on any FunctionOpInterface operations.
+    /// TODO Remove this once there is an alternative way of modeling memory
+    /// effects on FunctionOpInterface.
+    /// Name of the attribute that will cause the creation of a readnone memory
+    /// effect when lowering to the LLVMDialect.
     static StringRef getReadnoneAttrName() {
       return "llvm.readnone";
     }
 
     Type parseType(DialectAsmParser &p) const override;
     void printType(Type, DialectAsmPrinter &p) const override;
 
   private:

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

let arguments = (ins
     DefaultValuedAttr<Linkage, "Linkage::External">:$linkage,
     UnitAttr:$dso_local,
     DefaultValuedAttr<CConv, "CConv::C">:$CConv,
     OptionalAttr<FlatSymbolRefAttr>:$personality,
     OptionalAttr<StrAttr>:$garbageCollector,
     OptionalAttr<ArrayAttr>:$passthrough,
     OptionalAttr<DictArrayAttr>:$arg_attrs,
     OptionalAttr<DictArrayAttr>:$res_attrs,
-    OptionalAttr<I64Attr>:$function_entry_count
+    OptionalAttr<I64Attr>:$function_entry_count,
+    OptionalAttr<LLVM_MemoryEffectsAttr>:$memory
   );
 
   let regions = (region AnyRegion:$body);
 
   let skipDefaultBuilders = 1;
 
   let builders = [
     OpBuilder<(ins "StringRef":$name, "Type":$type,

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

 /// attributes.
 static void filterFuncAttributes(func::FuncOp func, bool filterArgAndResAttrs,
                                  SmallVectorImpl<NamedAttribute> &result) {
   for (const NamedAttribute &attr : func->getAttrs()) {
     if (attr.getName() == SymbolTable::getSymbolAttrName() ||
         attr.getName() == func.getFunctionTypeAttrName() ||
         attr.getName() == linkageAttrName ||
         attr.getName() == varargsAttrName ||
+        attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName() ||
         (filterArgAndResAttrs &&
          (attr.getName() == func.getArgAttrsAttrName() ||
           attr.getName() == func.getResAttrsAttrName())))
       continue;
     result.push_back(attr);
   }
 }
 
if (funcOp->hasAttr(linkageAttrName)) {
           funcOp->getAttr(linkageAttrName).dyn_cast<mlir::LLVM::LinkageAttr>();
       if (!attr) {
         funcOp->emitError() << "Contains " << linkageAttrName
                             << " attribute not of type LLVM::LinkageAttr";
         return nullptr;
       }
       linkage = attr.getLinkage();
     }
+
+    // Create a memory effect attribute corresponding to readnone.
+    StringRef readnoneAttrName = LLVM::LLVMDialect::getReadnoneAttrName();
+    LLVM::MemoryEffectsAttr memoryAttr = {};
+    if (funcOp->hasAttr(readnoneAttrName)) {
+      auto attr = funcOp->getAttrOfType<UnitAttr>(readnoneAttrName);
+      if (!attr) {
+        funcOp->emitError() << "Contains " << readnoneAttrName
+                            << " attribute not of type UnitAttr";
+        return nullptr;
+      }
+      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);
+    // 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;
 
     return newFuncOp;
   }

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

auto parseLoopOptions = [&]() -> ParseResult {
     return success();
   };
   if (parser.parseCommaSeparatedList(parseLoopOptions) || parser.parseGreater())
     return {};
 
   llvm::sort(options, llvm::less_first());
   return get(parser.getContext(), options);
 }
+
+//===----------------------------------------------------------------------===//
+// MemoryEffectsAttr
+//===----------------------------------------------------------------------===//
+
+MemoryEffectsAttr MemoryEffectsAttr::get(MLIRContext *context,
+                                         ArrayRef<ModRefInfo> memInfoArgs) {
+  if (memInfoArgs.empty())
+    return MemoryEffectsAttr::get(context, ModRefInfo::ModRef,
+                                  ModRefInfo::ModRef, ModRefInfo::ModRef);
+  if (memInfoArgs.size() == 3)
+    return MemoryEffectsAttr::get(context, memInfoArgs[0], memInfoArgs[1],
+                                  memInfoArgs[2]);
+  return {};
+}
+
+bool MemoryEffectsAttr::isReadWrite() {
+  if (this->getArgMem() != ModRefInfo::ModRef)
+    return false;
+  if (this->getInaccessibleMem() != ModRefInfo::ModRef)
+    return false;
+  if (this->getOther() != ModRefInfo::ModRef)
+    return false;
+  return true;
+}

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

if (attr.getName() == LLVMDialect::getLoopAttrName()) {
     std::optional<NamedAttribute> loopOptions =
         loopAttr.getNamed(LLVMDialect::getLoopOptionsAttrName());
     if (loopOptions && !loopOptions->getValue().isa<LoopOptionsAttr>())
       return op->emitOpError()
              << "expected '" << LLVMDialect::getLoopOptionsAttrName()
              << "' to be a `loopopts` attribute";
   }
 
-  if (attr.getName() == LLVMDialect::getReadnoneAttrName()) {
-    const auto attrName = LLVMDialect::getReadnoneAttrName();
-    if (!isa<FunctionOpInterface>(op))
-      return op->emitOpError()
-             << "'" << attrName
-             << "' is permitted only on FunctionOpInterface operations";
-    if (!attr.getValue().isa<UnitAttr>())
-      return op->emitOpError()
-             << "expected '" << attrName << "' to be a unit attribute";
-  }
-
   if (attr.getName() == LLVMDialect::getStructAttrsAttrName()) {
     return op->emitOpError()
            << "'" << LLVM::LLVMDialect::getStructAttrsAttrName()
            << "' is permitted only in argument or result attributes";
   }
 
   // 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

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

 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
+#include "llvm/Support/ModRef.h"
 
 using namespace mlir;
 using namespace mlir::LLVM;
 using namespace mlir::LLVM::detail;
 
 #include "mlir/Dialect/LLVMIR/LLVMConversionEnumsFromLLVM.inc"
 
 /// Returns true if the LLVM IR intrinsic is convertible to an MLIR LLVM dialect

mlir/lib/Target/LLVMIR/ModuleImport.cpp

 #include "llvm/ADT/StringSet.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/Support/ModRef.h"
 
 using namespace mlir;
 using namespace mlir::LLVM;
 using namespace mlir::LLVM::detail;
 
 #include "mlir/Dialect/LLVMIR/LLVMConversionEnumsFromLLVM.inc"
 
 // Utility to print an LLVM value as a string for passing to emitError().
if (ce->getOpcode() == llvm::Instruction::BitCast &&
         ce->getType() == llvm::Type::getInt8PtrTy(f->getContext())) {
       if (auto *func = dyn_cast<llvm::Function>(ce->getOperand(0)))
         return SymbolRefAttr::get(builder.getContext(), func->getName());
     }
   }
   return FlatSymbolRefAttr();
 }
 
+static void processMemoryEffects(llvm::Function *func, LLVMFuncOp funcOp) {
+  llvm::MemoryEffects memEffects = func->getMemoryEffects();
+
+  auto othermem = convertModRefInfoFromLLVM(
+      memEffects.getModRef(llvm::MemoryEffects::Location::Other));
+  auto argMem = convertModRefInfoFromLLVM(
+      memEffects.getModRef(llvm::MemoryEffects::Location::ArgMem));
+  auto inaccessibleMem = convertModRefInfoFromLLVM(
+      memEffects.getModRef(llvm::MemoryEffects::Location::InaccessibleMem));
+  auto memAttr = MemoryEffectsAttr::get(funcOp.getContext(), othermem, argMem,
+                                        inaccessibleMem);
+  // Only set the attr when it does not match the default value.
+  if (memAttr.isReadWrite())
+    return;
+  funcOp.setMemoryAttr(memAttr);
+}
+
 void ModuleImport::processFunctionAttributes(llvm::Function *func,
                                              LLVMFuncOp funcOp) {
-  auto addNamedUnitAttr = [&](StringRef name) {
-    return funcOp->setAttr(name, UnitAttr::get(context));
-  };
-  if (func->doesNotAccessMemory())
-    addNamedUnitAttr(LLVMDialect::getReadnoneAttrName());
+  processMemoryEffects(func, funcOp);
 }
 
 LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   clearBlockAndValueMapping();
 
   auto functionType =
       convertType(func->getFunctionType()).dyn_cast<LLVMFunctionType>();
   if (func->isIntrinsic() &&

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

 
 LogicalResult ModuleTranslation::convertDialectAttributes(Operation *op) {
   for (NamedAttribute attribute : op->getDialectAttrs())
     if (failed(iface.amendOperation(op, attribute, *this)))
       return failure();
   return success();
 }
 
+/// Converts the function attributes from LLVMFuncOp and attaches them to the
+/// llvm::Function.
+static void convertFunctionAttributes(LLVMFuncOp func,
+                                      llvm::Function *llvmFunc) {
+  if (!func.getMemory())
+    return;
+
+  MemoryEffectsAttr memEffects = func.getMemoryAttr();
+
+  // Add memory effects incrementally.
+  llvm::MemoryEffects newMemEffects =
+      llvm::MemoryEffects(llvm::MemoryEffects::Location::ArgMem,
+                          convertModRefInfoToLLVM(memEffects.getArgMem()));
+  newMemEffects |= llvm::MemoryEffects(
+      llvm::MemoryEffects::Location::InaccessibleMem,
+      convertModRefInfoToLLVM(memEffects.getInaccessibleMem()));
+  newMemEffects |=
+      llvm::MemoryEffects(llvm::MemoryEffects::Location::Other,
+                          convertModRefInfoToLLVM(memEffects.getOther()));
+  llvmFunc->setMemoryEffects(newMemEffects);
+}
+
 LogicalResult ModuleTranslation::convertFunctionSignatures() {
   // Declare all functions first because there may be function calls that form a
   // call graph with cycles, or global initializers that reference functions.
   for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {
     llvm::FunctionCallee llvmFuncCst = llvmModule->getOrInsertFunction(
         function.getName(),
         cast<llvm::FunctionType>(convertType(function.getFunctionType())));
     llvm::Function *llvmFunc = cast<llvm::Function>(llvmFuncCst.getCallee());
     llvmFunc->setLinkage(convertLinkageToLLVM(function.getLinkage()));
     llvmFunc->setCallingConv(convertCConvToLLVM(function.getCConv()));
     mapFunction(function.getName(), llvmFunc);
     addRuntimePreemptionSpecifier(function.getDsoLocal(), llvmFunc);
 
     // Convert function attributes.
-    if (function->getAttrOfType<UnitAttr>(LLVMDialect::getReadnoneAttrName()))
-      llvmFunc->setDoesNotAccessMemory();
+    convertFunctionAttributes(function, llvmFunc);
 
     // Convert function_entry_count attribute to metadata.
     if (std::optional<uint64_t> entryCount = function.getFunctionEntryCount())
       llvmFunc->setEntryCount(entryCount.value());
 
     // Convert result attributes.
     if (ArrayAttr allResultAttrs = function.getAllResultAttrs()) {
       llvm::AttrBuilder retAttrs(llvmFunc->getContext());
       DictionaryAttr resultAttrs = allResultAttrs[0].cast<DictionaryAttr>();

mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir

 // CHECK-NEXT:  llvm.return %0 : !llvm.ptr<func<void ()>>
   return %bbarg : () -> ()
 }
 
 // CHECK-LABEL: llvm.func extern_weak @llvmlinkage(i32)
 func.func private @llvmlinkage(i32) attributes { "llvm.linkage" = #llvm.linkage<extern_weak> }
 
 // CHECK-LABEL: llvm.func @llvmreadnone(i32)
-// CHECK-SAME: llvm.readnone
+// CHECK-SAME: memory = #llvm.memory_effects<other = none, argMem = none, inaccessibleMem = none>
 func.func private @llvmreadnone(i32) attributes { llvm.readnone }
 
 // CHECK-LABEL: llvm.func @body(i32)
 func.func private @body(i32)
 
 // CHECK-LABEL: llvm.func @indirect_const_call
 // CHECK-SAME: (%[[ARG0:.*]]: i32) {
 func.func @indirect_const_call(%arg0: i32) {

mlir/test/Dialect/LLVMIR/func.mlir

module {
   llvm.func weak fastcc @cconv3() {
     llvm.return
   }
 
   // CHECK-LABEL: llvm.func @variadic_def
   llvm.func @variadic_def(...) {
     llvm.return
   }
+
+  // CHECK-LABEL: llvm.func @memory_attr
+  // CHECK-SAME: attributes {memory = #llvm.memory_effects<other = none, argMem = read, inaccessibleMem = readwrite>} {
+  llvm.func @memory_attr() attributes {memory = #llvm.memory_effects<other = none, argMem = read, inaccessibleMem = readwrite>} {
+    llvm.return
+  }
 }
 
 // -----
 
 module {
   // expected-error@+1 {{requires one region}}
   "llvm.func"() {function_type = !llvm.func<void ()>, sym_name = "no_region"} : () -> ()
 }

 // -----
 
 module {
   "llvm.func"() ({
   // expected-error @below {{invalid Calling Conventions specification: cc_12}}
   // expected-error @below {{failed to parse CConvAttr parameter 'CallingConv' which is to be a `CConv`}}
   }) {sym_name = "generic_unknown_calling_convention", CConv = #llvm.cconv<cc_12>, function_type = !llvm.func<i64 (i64, i64)>} : () -> ()
 }
-
-// -----
-
-module {
-  // expected-error@+3 {{'llvm.readnone' is permitted only on FunctionOpInterface operations}}
-  "llvm.func"() ({
-  ^bb0:
-    llvm.return {llvm.readnone}
-  }) {sym_name = "readnone_return", function_type = !llvm.func<void ()>} : () -> ()
-}
-
-// -----
-
-module {
-  // expected-error@+1 {{op expected 'llvm.readnone' to be a unit attribute}}
-  "llvm.func"() ({
-  }) {sym_name = "readnone_func", llvm.readnone = true, function_type = !llvm.func<void ()>} : () -> ()
-}

mlir/test/Target/LLVMIR/Import/function-attributes.ll

 ; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
 ; CHECK: llvm.func internal @func_internal
 define internal void @func_internal() {
   ret void
 }
 
 ; CHECK: llvm.func internal spir_funccc @spir_func_internal()
 define internal spir_func void @spir_func_internal() {
   ret void
 }
 
 ; // -----
 
 ; CHECK-LABEL: @func_readnone
-; CHECK-SAME:  attributes {llvm.readnone}
+; CHECK-SAME:  attributes {memory = #llvm.memory_effects<other = none, argMem = none, inaccessibleMem = none>}
 ; CHECK:   llvm.return
 define void @func_readnone() readnone {
   ret void
 }
 
 ; CHECK-LABEL: @func_readnone_indirect
-; CHECK-SAME:  attributes {llvm.readnone}
+; CHECK-SAME:  attributes {memory = #llvm.memory_effects<other = none, argMem = none, inaccessibleMem = none>}
 declare void @func_readnone_indirect() #0
 attributes #0 = { readnone }
 
 ; // -----
 
 ; CHECK-LABEL: @func_arg_attrs
 ; CHECK-SAME:  !llvm.ptr {llvm.byval = i64}
 ; CHECK-SAME:  !llvm.ptr {llvm.byref = i64}

 
 ; CHECK-LABEL: @entry_count
 ; CHECK-SAME:  attributes {function_entry_count = 4242 : i64}
 define void @entry_count() !prof !1 {
   ret void
 }
 
 !1 = !{!"function_entry_count", i64 4242}
+
+; // -----
+
+; CHECK-LABEL: @func_memory
+; CHECK-SAME:  attributes {memory = #llvm.memory_effects<other = readwrite, argMem = none, inaccessibleMem = readwrite>}
+; CHECK:   llvm.return
+define void @func_memory() memory(readwrite, argmem: none) {
+  ret void
+}

mlir/test/Target/LLVMIR/llvmir.mlir

llvm.func @vararg_function(%arg0: i32, ...) {
   llvm.intr.vaend %5
   llvm.intr.vaend %3
   // CHECK: ret void
   llvm.return
 }
 
 // -----
 
-// Function attributes: readnone
+// CHECK: declare void @readonly_function([[PTR:.+]] readonly)
+llvm.func @readonly_function(%arg0: !llvm.ptr<f32> {llvm.readonly})
+
+// -----
+
+// CHECK: declare void @arg_mem_none_func() #[[ATTR:[0-9]+]]
+llvm.func @arg_mem_none_func() attributes {
+  memory = #llvm.memory_effects<other = readwrite, argMem = none, inaccessibleMem = readwrite>}
 
-// CHECK: declare void @readnone_function() #[[ATTR:[0-9]+]]
-// CHECK: attributes #[[ATTR]] = { memory(none) }
-llvm.func @readnone_function() attributes {llvm.readnone}
+// CHECK: attributes #[[ATTR]] = { memory(readwrite, argmem: none) }
 
 // -----
-// CHECK: declare void @readonly_function([[PTR:.+]] readonly)
-llvm.func @readonly_function(%arg0: !llvm.ptr<f32> {llvm.readonly})
+
+// CHECK: declare void @readwrite_func() #[[ATTR:[0-9]+]]
+llvm.func @readwrite_func() attributes {
+  memory = #llvm.memory_effects<other = readwrite, argMem = readwrite, inaccessibleMem = readwrite>}
+
+// CHECK: attributes #[[ATTR]] = { memory(readwrite) }