[mlir][llvm] Improve error message when translating `llvm.call_intrinsic`

[Mogball]

Summary

This is more user-friendly over an opaque crash.

[Mogball]

add newline

[lattner]

Nice, thank you!

[rriddle]

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

62

Can you use mlir::debugString for this?

[Mogball]

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

62

It works on LLVM types?

[rriddle]

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

62

IIRC it's a generally thing, would need to double check though (on mobile right now)

[Mogball]

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

62

I looked around yet could not find one. Seems LLVM typically uses the stream operator but it's only overloaded for raw_ostream.

[Mogball]

rebase

[mehdi_amini]

Sorry had to revert because seems like it broke the build: https://lab.llvm.org/buildbot/#/builders/61/builds/46712

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

1786

Why did you need to move it? The only change I spot is the asm format. By the way why changing the format here?

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

2830

I'm confused: where is this coming from? I can't relate to the rest of the changes

[Mogball]

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

1786

The op was printing its attributes twice (since LLVMOpAttrs printer doesn't know to omit $intrin). Also, this operation is more of a generic op rather than an intrinsic op.

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

2830

Aargh I rebased incorrectly

mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td

uint64_t getSrcVectorBitWidth() {
       return getVectorBitWidth(getSrcvec().getType());
     }
     uint64_t getResVectorBitWidth() {
       return getVectorBitWidth(getRes().getType());
     }
   }];
 }
 
-//===--------------------------------------------------------------------===//
-// CallIntrinsicOp
-//===--------------------------------------------------------------------===//
-
-def LLVM_CallIntrinsicOp
-    : LLVM_Op<"call_intrinsic",
-              [DeclareOpInterfaceMethods<FastmathFlagsInterface>]> {
-  let summary = "Call to an LLVM intrinsic function.";
-  let description = [{
-    Call the specified llvm intrinsic. If the intrinsic is overloaded, use
-    the MLIR function type of this op to determine which intrinsic to call.
-    }];
-  let arguments = (ins StrAttr:$intrin, Variadic<LLVM_Type>:$args,
-                       DefaultValuedAttr<LLVM_FastmathFlagsAttr,
-                                         "{}">:$fastmathFlags);
-  let results = (outs Variadic<LLVM_Type>:$results);
-  let llvmBuilder = [{
-    return convertCallLLVMIntrinsicOp(op, builder, moduleTranslation);
-  }];
-  let assemblyFormat = [{
-    $intrin `(` $args `)` `:` functional-type($args, $results)
-      custom<LLVMOpAttrs>(attr-dict)
-  }];
-}
-
 //
 // LLVM Vector Predication operations.
 //
 
 class LLVM_VPBinaryBase<string mnem, Type element>
     : LLVM_OneResultIntrOp<"vp." # mnem, [0], [], [Pure]>,
       Arguments<(ins LLVM_VectorOf<element>:$lhs, LLVM_VectorOf<element>:$rhs,
                      LLVM_VectorOf<I1>:$mask, I32:$evl)>;

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

def LLVM_InlineAsmOp : LLVM_Op<"inline_asm", []> {
 
   let extraClassDeclaration = [{
     static StringRef getElementTypeAttrName() {
       return "elementtype";
     }
   }];
 }
 
+//===--------------------------------------------------------------------===//
+// CallIntrinsicOp
+//===--------------------------------------------------------------------===//
+
+def LLVM_CallIntrinsicOp
+    : LLVM_Op<"call_intrinsic",
+              [DeclareOpInterfaceMethods<FastmathFlagsInterface>]> {
+  let summary = "Call to an LLVM intrinsic function.";
+  let description = [{
+    Call the specified llvm intrinsic. If the intrinsic is overloaded, use
+    the MLIR function type of this op to determine which intrinsic to call.
+  }];
+  let arguments = (ins StrAttr:$intrin, Variadic<LLVM_Type>:$args,
+                       DefaultValuedAttr<LLVM_FastmathFlagsAttr,
+                                         "{}">:$fastmathFlags);
+  let results = (outs Optional<LLVM_Type>:$results);
+  let llvmBuilder = [{
+    return convertCallLLVMIntrinsicOp(op, builder, moduleTranslation);
+  }];
+  let assemblyFormat = [{
+    $intrin `(` $args `)` `:` functional-type($args, $results) attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
 #endif // LLVMIR_OPS

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

 //===- LLVMDialect.cpp - LLVM IR Ops and Dialect registration -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the types and operation details for the LLVM IR dialect in
 // MLIR, and the LLVM IR dialect.  It also registers the dialect.
 //
 //===----------------------------------------------------------------------===//
+
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "LLVMInlining.h"
 #include "TypeDetail.h"
 #include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
 #include "mlir/Dialect/LLVMIR/LLVMInterfaces.h"
 #include "mlir/Dialect/LLVMIR/LLVMTypes.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinOps.h"
OpFoldResult LLVM::OrOp::fold(FoldAdaptor adaptor) {
   auto rhs = dyn_cast_or_null<IntegerAttr>(adaptor.getRhs());
   if (!rhs)
     return {};
 
   return IntegerAttr::get(getType(), lhs.getValue() | rhs.getValue());
 }
 
 //===----------------------------------------------------------------------===//
+// Utilities for LLVM::MetadataOp
+//===----------------------------------------------------------------------===//
+
+void MetadataOp::build(OpBuilder &builder, OperationState &result,
+                       StringRef symName, bool createBodyBlock,
+                       ArrayRef<NamedAttribute> attributes) {
+  result.addAttribute(getSymNameAttrName(result.name),
+                      builder.getStringAttr(symName));
+  result.attributes.append(attributes.begin(), attributes.end());
+  Region *body = result.addRegion();
+  if (createBodyBlock)
+    body->emplaceBlock();
+}
+
+ParseResult MetadataOp::parse(OpAsmParser &parser, OperationState &result) {
+  StringAttr symName;
+  if (parser.parseSymbolName(symName, getSymNameAttrName(result.name),
+                             result.attributes) ||
+      parser.parseOptionalAttrDictWithKeyword(result.attributes))
+    return failure();
+
+  Region *bodyRegion = result.addRegion();
+  if (parser.parseRegion(*bodyRegion))
+    return failure();
+
+  // If the region appeared to be empty to parseRegion(),
+  // add the body block explicitly.
+  if (bodyRegion->empty())
+    bodyRegion->emplaceBlock();
+
+  return success();
+}
+
+void MetadataOp::print(OpAsmPrinter &printer) {
+  printer << ' ';
+  printer.printSymbolName(getSymName());
+  printer.printOptionalAttrDictWithKeyword((*this)->getAttrs(),
+                                           {getSymNameAttrName().getValue()});
+  printer << ' ';
+  printer.printRegion(getBody());
+}
+
+//===----------------------------------------------------------------------===//
+// CallIntrinsicOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult CallIntrinsicOp::verify() {
+  if (!getIntrin().startswith("llvm."))
+    return emitOpError() << "intrinsic name must start with 'llvm.'";
+  return success();
+}
+
+//===----------------------------------------------------------------------===//
 // OpAsmDialectInterface
 //===----------------------------------------------------------------------===//
 
 namespace {
 struct LLVMOpAsmDialectInterface : public OpAsmDialectInterface {
   using OpAsmDialectInterface::OpAsmDialectInterface;
 
   AliasResult getAlias(Attribute attr, raw_ostream &os) const override {

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

 
 /// Convert the value of a DenseI64ArrayAttr to a vector of unsigned indices.
 static SmallVector<unsigned> extractPosition(ArrayRef<int64_t> indices) {
   SmallVector<unsigned> position;
   llvm::append_range(position, indices);
   return position;
 }
 
+/// Convert an LLVM type to a string for printing in diagnostics.
+static std::string diagStr(const llvm::Type *type) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+  type->print(os);
+  return os.str();
+}
+
 /// Get the declaration of an overloaded llvm intrinsic. First we get the
 /// overloaded argument types and/or result type from the CallIntrinsicOp, and
 /// then use those to get the correct declaration of the overloaded intrinsic.
 static FailureOr<llvm::Function *>
-getOverloadedDeclaration(CallIntrinsicOp &op, llvm::Intrinsic::ID id,
+getOverloadedDeclaration(CallIntrinsicOp op, llvm::Intrinsic::ID id,
                          llvm::Module *module,
                          LLVM::ModuleTranslation &moduleTranslation) {
   SmallVector<llvm::Type *, 8> allArgTys;
   for (Type type : op->getOperandTypes())
     allArgTys.push_back(moduleTranslation.convertType(type));
 
   llvm::Type *resTy;
   if (op.getNumResults() == 0)
     resTy = llvm::Type::getVoidTy(module->getContext());
   else
     resTy = moduleTranslation.convertType(op.getResult(0).getType());
 
   // ATM we do not support variadic intrinsics.
   llvm::FunctionType *ft = llvm::FunctionType::get(resTy, allArgTys, false);
 
   SmallVector<llvm::Intrinsic::IITDescriptor, 8> table;
   getIntrinsicInfoTableEntries(id, table);
   ArrayRef<llvm::Intrinsic::IITDescriptor> tableRef = table;
 
   SmallVector<llvm::Type *, 8> overloadedArgTys;
   if (llvm::Intrinsic::matchIntrinsicSignature(ft, tableRef,
                                                overloadedArgTys) !=
       llvm::Intrinsic::MatchIntrinsicTypesResult::MatchIntrinsicTypes_Match) {
-    return op.emitOpError("intrinsic type is not a match");
+    return mlir::emitError(op.getLoc(), "call intrinsic signature ")
+           << diagStr(ft) << " to overloaded intrinsic " << op.getIntrinAttr()
+           << " does not match any of the overloads";
   }
 
   ArrayRef<llvm::Type *> overloadedArgTysRef = overloadedArgTys;
   return llvm::Intrinsic::getDeclaration(module, id, overloadedArgTysRef);
 }
 
 /// Builder for LLVM_CallIntrinsicOp
 static LogicalResult
 convertCallLLVMIntrinsicOp(CallIntrinsicOp op, llvm::IRBuilderBase &builder,
                            LLVM::ModuleTranslation &moduleTranslation) {
   llvm::Module *module = builder.GetInsertBlock()->getModule();
   llvm::Intrinsic::ID id =
       llvm::Function::lookupIntrinsicID(op.getIntrinAttr());
   if (!id)
-    return op.emitOpError()
-           << "couldn't find intrinsic: " << op.getIntrinAttr();
+    return mlir::emitError(op.getLoc(), "could not find LLVM intrinsic: ")
+           << op.getIntrinAttr();
 
   llvm::Function *fn = nullptr;
   if (llvm::Intrinsic::isOverloaded(id)) {
     auto fnOrFailure =
         getOverloadedDeclaration(op, id, module, moduleTranslation);
     if (failed(fnOrFailure))
       return failure();
     fn = *fnOrFailure;
   } else {
     fn = llvm::Intrinsic::getDeclaration(module, id, {});
   }
+
+  // Check the result type of the call.
+  const llvm::Type *intrinType =
+      op.getNumResults() == 0
+          ? llvm::Type::getVoidTy(module->getContext())
+          : moduleTranslation.convertType(op.getResultTypes().front());
+  if (intrinType != fn->getReturnType()) {
+    return mlir::emitError(op.getLoc(), "intrinsic call returns ")
+           << diagStr(intrinType) << " but " << op.getIntrinAttr()
+           << " actually returns " << diagStr(fn->getReturnType());
+  }
+
+  // Check the argument types of the call. If the function is variadic, check
+  // the subrange of required arguments.
+  if (!fn->getFunctionType()->isVarArg() &&
+      op.getNumOperands() != fn->arg_size()) {
+    return mlir::emitError(op.getLoc(), "intrinsic call has ")
+           << op.getNumOperands() << " operands but " << op.getIntrinAttr()
+           << " expects " << fn->arg_size();
+  }
+  if (fn->getFunctionType()->isVarArg() &&
+      op.getNumOperands() < fn->arg_size()) {
+    return mlir::emitError(op.getLoc(), "intrinsic call has ")
+           << op.getNumOperands() << " operands but variadic "
+           << op.getIntrinAttr() << " expects at least " << fn->arg_size();
+  }
+  // Check the arguments up to the number the function requires.
+  for (unsigned i = 0, e = fn->arg_size(); i != e; ++i) {
+    const llvm::Type *expected = fn->getArg(i)->getType();
+    const llvm::Type *actual =
+        moduleTranslation.convertType(op.getOperandTypes()[i]);
+    if (actual != expected) {
+      return mlir::emitError(op.getLoc(), "intrinsic call operand #")
+             << i << " has type " << diagStr(actual) << " but "
+             << op.getIntrinAttr() << " expects " << diagStr(expected);
+    }
+  }
+
   FastmathFlagsInterface itf = op;
   builder.setFastMathFlags(getFastmathFlags(itf));
 
   auto *inst =
       builder.CreateCall(fn, moduleTranslation.lookupValues(op.getOperands()));
   if (op.getNumResults() == 1)
     moduleTranslation.mapValue(op->getResults().front()) = inst;
   return success();

mlir/test/Dialect/LLVMIR/call-intrin.mlir

 // RUN: mlir-translate -mlir-to-llvmir -split-input-file -verify-diagnostics %s | FileCheck %s
 
-// CHECK: ; ModuleID = 'LLVMDialectModule'
-// CHECK: source_filename = "LLVMDialectModule"
-// CHECK: declare ptr @malloc(i64)
-// CHECK: declare void @free(ptr)
 // CHECK: define <4 x float> @round_sse41() {
-// CHECK:  %1 = call reassoc <4 x float> @llvm.x86.sse41.round.ss(<4 x float> <float 0x3FC99999A0000000, float 0x3FC99999A0000000, float 0x3FC99999A0000000, float 0x3FC99999A0000000>, <4 x float> <float 0x3FC99999A0000000, float 0x3FC99999A0000000, float 0x3FC99999A0000000, float 0x3FC99999A0000000>, i32 1)
+// CHECK:  %1 = call reassoc <4 x float> @llvm.x86.sse41.round.ss(<4 x float> {{.*}}, <4 x float> {{.*}}, i32 1)
 // CHECK:  ret <4 x float> %1
 // CHECK: }
 llvm.func @round_sse41() -> vector<4xf32> {
-    %0 = llvm.mlir.constant(1 : i32) : i32
-    %1 = llvm.mlir.constant(dense<0.2> : vector<4xf32>) : vector<4xf32>
-    %res = llvm.call_intrinsic "llvm.x86.sse41.round.ss"(%1, %1, %0) : (vector<4xf32>, vector<4xf32>, i32) -> vector<4xf32> {fastmathFlags = #llvm.fastmath<reassoc>}
-    llvm.return %res: vector<4xf32>
 }
 
 // -----
 
-// CHECK: ; ModuleID = 'LLVMDialectModule'
-// CHECK: source_filename = "LLVMDialectModule"
-
-// CHECK: declare ptr @malloc(i64)
-
-// CHECK: declare void @free(ptr)
-
 // CHECK: define float @round_overloaded() {
 // CHECK:   %1 = call float @llvm.round.f32(float 1.000000e+00)
 // CHECK:   ret float %1
 // CHECK: }
 llvm.func @round_overloaded() -> f32 {
-    %0 = llvm.mlir.constant(1.0 : f32) : f32
-    %res = llvm.call_intrinsic "llvm.round"(%0) : (f32) -> f32 {}
-    llvm.return %res: f32
 }
 
 // -----
 
-// CHECK: ; ModuleID = 'LLVMDialectModule'
-// CHECK: source_filename = "LLVMDialectModule"
-// CHECK: declare ptr @malloc(i64)
-// CHECK: declare void @free(ptr)
 // CHECK: define void @lifetime_start() {
 // CHECK:   %1 = alloca float, i8 1, align 4
 // CHECK:   call void @llvm.lifetime.start.p0(i64 4, ptr %1)
 // CHECK:   ret void
 // CHECK: }
 llvm.func @lifetime_start() {
-    %0 = llvm.mlir.constant(4 : i64) : i64
-    %1 = llvm.mlir.constant(1 : i8) : i8
-    %2 = llvm.alloca %1 x f32 : (i8) -> !llvm.ptr
-    llvm.call_intrinsic "llvm.lifetime.start"(%0, %2) : (i64, !llvm.ptr) -> () {}
-    llvm.return
 }
 
 // -----
 
+// CHECK-LABEL: define void @variadic()
 llvm.func @variadic() {
-    %0 = llvm.mlir.constant(1 : i8) : i8
-    %1 = llvm.alloca %0 x f32 : (i8) -> !llvm.ptr
-    llvm.call_intrinsic "llvm.localescape"(%1, %1) : (!llvm.ptr, !llvm.ptr) -> ()
-    llvm.return
+  // CHECK: call void (...) @llvm.localescape(ptr %1, ptr %1)
 }
 
 // -----
 
 llvm.func @no_intrinsic() {
-    // expected-error@below {{'llvm.call_intrinsic' op couldn't find intrinsic: "llvm.does_not_exist"}}
-    // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
-    llvm.call_intrinsic "llvm.does_not_exist"() : () -> ()
-    llvm.return
+  // expected-error@below {{could not find LLVM intrinsic: "llvm.does_not_exist"}}
 }
 
 // -----
 
 llvm.func @bad_types() {
-    %0 = llvm.mlir.constant(1 : i8) : i8
-    // expected-error@below {{'llvm.call_intrinsic' op intrinsic type is not a match}}
-    // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
-    llvm.call_intrinsic "llvm.round"(%0) : (i8) -> i8 {}
-    llvm.return
+  // expected-error@below {{call intrinsic signature i8 (i8) to overloaded intrinsic "llvm.round" does not match any of the overloads}}
 }
+
+// -----
+
+llvm.func @bad_result() {
+  // expected-error @below {{intrinsic call returns void but "llvm.x86.sse41.round.ss" actually returns <4 x float>}}
+  // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
+  llvm.call_intrinsic "llvm.x86.sse41.round.ss"() : () -> ()
+  llvm.return
+}
+
+// -----
+
+llvm.func @bad_result() {
+  // expected-error @below {{intrinsic call returns <8 x float> but "llvm.x86.sse41.round.ss" actually returns <4 x float>}}
+  // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
+  llvm.call_intrinsic "llvm.x86.sse41.round.ss"() : () -> (vector<8xf32>)
+  llvm.return
+}
+
+// -----
+
+llvm.func @bad_args() {
+  // expected-error @below {{intrinsic call has 0 operands but "llvm.x86.sse41.round.ss" expects 3}}
+  // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
+  llvm.call_intrinsic "llvm.x86.sse41.round.ss"() : () -> (vector<4xf32>)
+  llvm.return
+}
+
+// -----
+
+llvm.func @bad_args() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.mlir.constant(dense<0.2> : vector<4xf32>) : vector<4xf32>
+  // expected-error @below {{intrinsic call operand #2 has type i64 but "llvm.x86.sse41.round.ss" expects i32}}
+  // expected-error@below {{LLVM Translation failed for operation: llvm.call_intrinsic}}
+  %res = llvm.call_intrinsic "llvm.x86.sse41.round.ss"(%1, %1, %0) : (vector<4xf32>, vector<4xf32>, i64) -> vector<4xf32> {fastmathFlags = #llvm.fastmath<reassoc>}
+  llvm.return
+}