From: Christian Ulmann <christian.ulmann@nextsilicon.com>
Date: Thu, 12 Jan 2023 15:48:19 +0000 (+0100)
Subject: [mlir][LLVM] Replace readnone with memory effects
X-Git-Tag: upstream/17.0.6~20464
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9b9cfe77a50abccc4b82a497e17566a454b699bd;p=platform%2Fupstream%2Fllvm.git

[mlir][LLVM] Replace readnone with memory effects

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

Differential Revision: https://reviews.llvm.org/D142013
---

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 4963bc9..96936c4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -373,4 +373,25 @@ def LLVM_DISubroutineTypeAttr : LLVM_Attr<"DISubroutineType", "di_subroutine_typ
   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
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index 45e03b3..17b372f 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -524,4 +524,21 @@ def UnnamedAddr : LLVM_EnumAttr<
   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
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
index 37365f6..eda30e3 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
@@ -75,8 +75,10 @@ def LLVM_Dialect : Dialect {
     /// 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";
     }
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 88eacda..c31245c 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1486,7 +1486,8 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     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);
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 552bb1e..b18bf56 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -69,6 +69,7 @@ static void filterFuncAttributes(func::FuncOp func, bool filterArgAndResAttrs,
         attr.getName() == func.getFunctionTypeAttrName() ||
         attr.getName() == linkageAttrName ||
         attr.getName() == varargsAttrName ||
+        attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName() ||
         (filterArgAndResAttrs &&
          (attr.getName() == func.getArgAttrsAttrName() ||
           attr.getName() == func.getResAttrsAttrName())))
@@ -374,9 +375,28 @@ protected:
       }
       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,
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 2f06d92..275f319 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -254,3 +254,28 @@ Attribute LoopOptionsAttr::parse(AsmParser &parser, Type type) {
   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;
+}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index a4c6568..955eb91 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2990,17 +2990,6 @@ LogicalResult LLVMDialect::verifyOperationAttribute(Operation *op,
              << "' 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()
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
index 24e5ab3..7d55eae 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
@@ -23,6 +23,7 @@
 #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;
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index a11ff55..859087a 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -32,6 +32,7 @@
 #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;
@@ -1405,13 +1406,26 @@ FlatSymbolRefAttr ModuleImport::getPersonalityAsAttr(llvm::Function *f) {
   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) {
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index f069917..ecb9908 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -874,6 +874,28 @@ LogicalResult ModuleTranslation::convertDialectAttributes(Operation *op) {
   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.
@@ -888,8 +910,7 @@ LogicalResult ModuleTranslation::convertFunctionSignatures() {
     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())
diff --git a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
index 8148e33..9db15b8 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
@@ -41,7 +41,7 @@ func.func @pass_through(%arg0: () -> ()) -> (() -> ()) {
 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)
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index dedf001..c4afd42 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -188,6 +188,12 @@ module {
   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
+  }
 }
 
 // -----
@@ -347,21 +353,3 @@ module {
   // 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 ()>} : () -> ()
-}
diff --git a/mlir/test/Target/LLVMIR/Import/function-attributes.ll b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
index 3fac482..4dd456e 100644
--- a/mlir/test/Target/LLVMIR/Import/function-attributes.ll
+++ b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
@@ -13,14 +13,14 @@ define internal spir_func void @spir_func_internal() {
 ; // -----
 
 ; 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 }
 
@@ -48,3 +48,12 @@ define void @entry_count() !prof !1 {
 }
 
 !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
+}
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 18331a2..dde4bb3 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -2067,12 +2067,21 @@ llvm.func @vararg_function(%arg0: i32, ...) {
 
 // -----
 
-// 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) }