diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td @@ -373,4 +373,25 @@ 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":$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 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td @@ -524,4 +524,21 @@ 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 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td @@ -75,8 +75,10 @@ /// 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 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -1486,7 +1486,8 @@ OptionalAttr:$passthrough, OptionalAttr:$arg_attrs, OptionalAttr:$res_attrs, - OptionalAttr:$function_entry_count + OptionalAttr:$function_entry_count, + OptionalAttr:$memory ); let regions = (region AnyRegion:$body); diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp --- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp +++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp @@ -69,6 +69,7 @@ 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 @@ } 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(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( 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 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp @@ -254,3 +254,28 @@ llvm::sort(options, llvm::less_first()); return get(parser.getContext(), options); } + +//===----------------------------------------------------------------------===// +// MemoryEffectsAttr +//===----------------------------------------------------------------------===// + +MemoryEffectsAttr MemoryEffectsAttr::get(MLIRContext *context, + ArrayRef 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 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -2990,17 +2990,6 @@ << "' to be a `loopopts` attribute"; } - if (attr.getName() == LLVMDialect::getReadnoneAttrName()) { - const auto attrName = LLVMDialect::getReadnoneAttrName(); - if (!isa(op)) - return op->emitOpError() - << "'" << attrName - << "' is permitted only on FunctionOpInterface operations"; - if (!attr.getValue().isa()) - 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 --- 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 --- 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 @@ 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 --- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp @@ -874,6 +874,28 @@ 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 @@ addRuntimePreemptionSpecifier(function.getDsoLocal(), llvmFunc); // Convert function attributes. - if (function->getAttrOfType(LLVMDialect::getReadnoneAttrName())) - llvmFunc->setDoesNotAccessMemory(); + convertFunctionAttributes(function, llvmFunc); // Convert function_entry_count attribute to metadata. if (std::optional entryCount = function.getFunctionEntryCount()) diff --git a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir --- a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir +++ b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir @@ -41,7 +41,7 @@ func.func private @llvmlinkage(i32) attributes { "llvm.linkage" = #llvm.linkage } // CHECK-LABEL: llvm.func @llvmreadnone(i32) -// CHECK-SAME: llvm.readnone +// CHECK-SAME: memory = #llvm.memory_effects 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 --- a/mlir/test/Dialect/LLVMIR/func.mlir +++ b/mlir/test/Dialect/LLVMIR/func.mlir @@ -188,6 +188,12 @@ llvm.func @variadic_def(...) { llvm.return } + + // CHECK-LABEL: llvm.func @memory_attr + // CHECK-SAME: attributes {memory = #llvm.memory_effects} { + llvm.func @memory_attr() attributes {memory = #llvm.memory_effects} { + llvm.return + } } // ----- @@ -347,21 +353,3 @@ // expected-error @below {{failed to parse CConvAttr parameter 'CallingConv' which is to be a `CConv`}} }) {sym_name = "generic_unknown_calling_convention", CConv = #llvm.cconv, function_type = !llvm.func} : () -> () } - -// ----- - -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} : () -> () -} - -// ----- - -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} : () -> () -} diff --git a/mlir/test/Target/LLVMIR/Import/function-attributes.ll b/mlir/test/Target/LLVMIR/Import/function-attributes.ll --- a/mlir/test/Target/LLVMIR/Import/function-attributes.ll +++ b/mlir/test/Target/LLVMIR/Import/function-attributes.ll @@ -13,14 +13,14 @@ ; // ----- ; CHECK-LABEL: @func_readnone -; CHECK-SAME: attributes {llvm.readnone} +; CHECK-SAME: attributes {memory = #llvm.memory_effects} ; 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} declare void @func_readnone_indirect() #0 attributes #0 = { readnone } @@ -48,3 +48,12 @@ } !1 = !{!"function_entry_count", i64 4242} + +; // ----- + +; CHECK-LABEL: @func_memory +; CHECK-SAME: attributes {memory = #llvm.memory_effects} +; 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 --- a/mlir/test/Target/LLVMIR/llvmir.mlir +++ b/mlir/test/Target/LLVMIR/llvmir.mlir @@ -2067,12 +2067,21 @@ // ----- -// Function attributes: readnone +// CHECK: declare void @readonly_function([[PTR:.+]] readonly) +llvm.func @readonly_function(%arg0: !llvm.ptr {llvm.readonly}) + +// ----- + +// CHECK: declare void @arg_mem_none_func() #[[ATTR:[0-9]+]] +llvm.func @arg_mem_none_func() attributes { + memory = #llvm.memory_effects} -// 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 {llvm.readonly}) + +// CHECK: declare void @readwrite_func() #[[ATTR:[0-9]+]] +llvm.func @readwrite_func() attributes { + memory = #llvm.memory_effects} + +// CHECK: attributes #[[ATTR]] = { memory(readwrite) }