[mlir] Support llvm.readnone attribute for all FunctionOpInterface ops.

[vzakhari]

Summary

The attribute is translated into LLVM's function attribute 'readnone'.
There is no explicit verification regarding conflicting 'readnone'
and function attributes from 'passthrough', though, LLVM would assert
if they are incompatible during LLVM IR creation.

[vzakhari]

Friendly ping. Please review.

[mehdi_amini]

Can you add a test for the import case?

[vzakhari]

In D131457#3733558, @mehdi_amini wrote:

Can you add a test for the import case?

Sure! Can you please elaborate on what the "import case" is? :)

[mehdi_amini]

Import is llvm IR -> MLIR instead of MLIR->LLVM IR

[vzakhari]

In D131457#3733576, @mehdi_amini wrote:

Import is llvm IR -> MLIR instead of MLIR->LLVM IR

Got it! Thanks! I will add a new test in mlir/test/Target/LLVMIR/Import/.

[vzakhari]

Okay, so this requires extending the "importer" (ConvertFromLLVMIR.cpp). Does it make sense to implement it in this differential or it would be better to do it separately?

[vzakhari]

Please let me know if I need to split the "importer" changes into a new differential. Otherwise, this should be ready for review. Thanks!

[mehdi_amini]

Thanks!

@ftynse ?

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

932

Nit: remove spurious braces.

[vzakhari]

Removed braces.

[vzakhari]

Kindly ping. Please review.

[vzakhari]

Thank you all for the review!

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.
+    static StringRef getReadnoneAttrName() {
+      return "llvm.readnone";
+    }
+
   private:
     /// A cache storing compatible LLVM types that have been verified. This
     /// can save us lots of verification time if there are many occurrences
     /// of some deeply-nested aggregate types in the program.
     ThreadLocalCache<DenseSet<Type>> compatibleTypes;
   }];
 
   let emitAccessorPrefix = kEmitAccessorPrefix_Prefixed;

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

if (attr.getName() == LLVMDialect::getLoopAttrName()) {
     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/ConvertFromLLVMIR.cpp

Importer(MLIRContext *context, ModuleOp module)
         unknownLoc(FileLineColLoc::get(context, "imported-bitcode", 0, 0)),
         typeTranslator(*context) {
     b.setInsertionPointToStart(module.getBody());
   }
 
   /// Imports `f` into the current module.
   LogicalResult processFunction(llvm::Function *f);
 
+  /// Converts function attributes of LLVM Function \p f
+  /// into LLVM dialect attributes of LLVMFuncOp \p funcOp.
+  void processFunctionAttributes(llvm::Function *f, LLVMFuncOp funcOp);
+
   /// Imports GV as a GlobalOp, creating it if it doesn't exist.
   GlobalOp processGlobal(llvm::GlobalVariable *gv);
 
 private:
   /// Returns personality of `f` as a FlatSymbolRefAttr.
   FlatSymbolRefAttr getPersonalityAsAttr(llvm::Function *f);
   /// Imports `bb` into `block`, which must be initially empty.
   LogicalResult processBasicBlock(llvm::BasicBlock *bb, Block *block);
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(b.getContext(), func->getName());
     }
   }
   return FlatSymbolRefAttr();
 }
 
+void Importer::processFunctionAttributes(llvm::Function *func,
+                                         LLVMFuncOp funcOp) {
+  auto addNamedUnitAttr = [&](StringRef name) {
+    return funcOp->setAttr(name, UnitAttr::get(context));
+  };
+  if (func->hasFnAttribute(llvm::Attribute::ReadNone))
+    addNamedUnitAttr(LLVMDialect::getReadnoneAttrName());
+}
+
 LogicalResult Importer::processFunction(llvm::Function *f) {
   blocks.clear();
   instMap.clear();
   unknownInstMap.clear();
 
   auto functionType =
       processType(f->getFunctionType()).dyn_cast<LLVMFunctionType>();
   if (!functionType)
if (FlatSymbolRefAttr personality = getPersonalityAsAttr(f))
     fop->setAttr(b.getStringAttr("personality"), personality);
   else if (f->hasPersonalityFn())
     emitWarning(UnknownLoc::get(context),
                 "could not deduce personality, skipping it");
 
   if (f->hasGC())
     fop.setGarbageCollectorAttr(b.getStringAttr(f->getGC()));
 
+  // Handle Function attributes.
+  processFunctionAttributes(f, fop);
+
   if (f->isDeclaration())
     return success();
 
   // Eagerly create all blocks.
   SmallVector<Block *, 4> blockList;
   for (llvm::BasicBlock &bb : *f) {
     blockList.push_back(b.createBlock(&fop.getBody(), fop.getBody().end()));
     blocks[&bb] = blockList.back();

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

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()));
     mapFunction(function.getName(), llvmFunc);
     addRuntimePreemptionSpecifier(function.getDsoLocal(), llvmFunc);
 
+    if (function->getAttrOfType<UnitAttr>(LLVMDialect::getReadnoneAttrName()))
+      llvmFunc->addFnAttr(llvm::Attribute::ReadNone);
+
     // Forward the pass-through attributes to LLVM.
     if (failed(forwardPassthroughAttributes(
             function.getLoc(), function.getPassthrough(), llvmFunc)))
       return failure();
   }
 
   return success();
 }

mlir/test/Dialect/LLVMIR/func.mlir

 
 // -----
 
 module {
   // expected-error@+2 {{unknown calling convention: cc_12}}
   "llvm.func"() ({
   }) {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 @readnone_attr() attributes {llvm.readnone}
+declare void @readnone_attr() #0
+attributes #0 = { readnone }
+
+// -----
+
+; CHECK: llvm.func @readnone_attr() attributes {llvm.readnone} {
+; CHECK:   llvm.return
+; CHECK: }
+define void @readnone_attr() readnone {
+entry:
+  ret void
+}

mlir/test/Target/LLVMIR/llvmir.mlir

llvm.func @vararg_function(%arg0: i32, ...) {
   llvm.intr.vacopy %3 to %5
   // CHECK: call void @llvm.va_end(ptr %[[ALLOCA1]])
   // CHECK: call void @llvm.va_end(ptr %[[ALLOCA0]])
   llvm.intr.vaend %5
   llvm.intr.vaend %3
   // CHECK: ret void
   llvm.return
 }
+
+// -----
+
+// Function attributes: readnone
+
+// CHECK: declare void @readnone_function() #[[ATTR:[0-9]+]]
+// CHECK: attributes #[[ATTR]] = { readnone }
+llvm.func @readnone_function() attributes {llvm.readnone}