[mlir] Add call_intrinsic op to LLVMIIR

[electriclilies]

Summary

The call_intrinsic op allows us to call LLVM intrinsics from the LLVMDialect without implementing a new op every time.

[Mogball]

nice

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

683–685

This is the new style for headers (LLVM dialect is very old!)

686

can you name this LLVM_CallIntrinsicOp? It affects the name of the C++ class

686

can you add a summary and description field to document the op?

692–694

701

can you outline this to a C++ file? I think you can add a function to mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp where the generated code gets pasted

[electriclilies]

Moved some logic from the builder into a function; responding to comments

[electriclilies]

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

701

I moved a bunch of it to getOverloadedDeclaration, let me know what you think

[Mogball]

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

697

An intrinsic that doesn't exist shouldn't cause the exporter to assert. Can you have it fail gracefully with an error?

703

please drop trivial braces

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

271

please drop trivial braces

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

15

What happens when you try to export an intrinsic that doesn't exist on all systems? I'm worried this test might fail on some systems

[lattner]

Very exciting Lily, thank you for implementing this!

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

266

Plz only eval getNumOperands once per iteration and prefer preincrement:

for (size_t i = 0, e = opInst.getNumOperands(); i != e; ++i)

It is a micro thing but helps with consistency.
https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop
https://llvm.org/docs/CodingStandards.html#prefer-preincrement

283

Do you need the llvm:: namespace qualifiers in the .cpp file? I think they can all be dropped

283

Can you pass Table directly into matchIntrinsicSignature? I think they implicitly convert and you can get rid of TableRef

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

15

I think it will always work but am not sure, how do we test this? Maybe add a weird target intrinsic that you don't have configured to check?

[electriclilies]

Fix error messages + comments

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

283

I do need the llvm:: namespace qualifiers unfortunately; a lot of other functions in this file use them too. :(

And I'm not sure why, but if I try to pass Table in directly, it doesn't compile with this error--

/home/lily/llvm-project/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp:283:52: error: non-const lvalue reference to type 'ArrayRef<llvm::Intrinsic::IITDescriptor>' cannot bind to a value of unrelated type 'SmallVector<llvm::Intrinsic::IITDescriptor, 8>'
  if (llvm::Intrinsic::matchIntrinsicSignature(FT, Table,

There are other functions that called matchIntrinsicSignature like this as well

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

15

I'm not 100% sure what the behavior is-- if the intrinsic isn't on a particular machine then then it might just return 0 when it tries to get the intrinsic. I can change this to a different test

15

Earlier today I was trying to use some intrinsics that were listed in llvmint, and getting back 0 for the id of those intrinsics. Turns out they were ones that weren't in ""LLVMConvertibleLLVMIRIntrinsics.inc".

I'm not sure if "LLVMConvertibleLLVMIRIntrinsics.inc" is dependent on the target or if it's just a static list that's the same across all backends.

[electriclilies]

Make errors a bit better

[electriclilies]

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

697

\

705

If this does fail, then there will probably be an error in > builder.CreateCall <. I'm not sure how to return early from a builder..

[electriclilies]

Make error reporting work properly

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

700

Currently this else is requ

705

Nevermind, I got this to work properly.

[lattner]

I do need the llvm:: namespace qualifiers unfortunately; a lot of other functions in this file use them too. :(

Ok!

[electriclilies]

clang-format

[rriddle]

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

689

To provide a similar summary to the normal call op.

690–692

698–699

700–701

How does success translate to error propagating correctly?

701

I would move all of this out of line if possible.

706–708

718

Drop the newline here.

725–727

I would actually advocate for not having this on the op definition, because it requires having LLVM IR linked and available (which I don't think we want really in the LLVM dialect, even if it currently does now).

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

261–262

Can you document this method? The contract seems specific, also can you have a more specific operation type for opInst?

286

Do you have tests for these error cases?

286–287

290–293

[electriclilies]

Comments + add more tests

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

700–701

It actually should be failure(), my error checking was not quite right and it was working for failure() and not success()

725–727

OK, I can inline it

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

261–262

It looks like opInst is the exact same thing as op, so I can actually just get rid of opInst.

286

I added a test for intrinsic type is not a match.

For the variadic-ness one, I can't find a variadic intrinsic, so I didn't add a test. It seems like variadic intrinsics are uncommon, though, and I'm not sure how to support them. I updated the error message to say that we don't support calling variadic intrinsics.

[electriclilies]

Move most of builder out of .td file

[electriclilies]

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

701

OK, I moved it all into another C++ function.

706–708

I removed the comment so I think I don't need the {} anymore

[electriclilies]

format

[rriddle]

As something that better enables scaling to the many intrinsics, this LGTM conceptually.

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

687–688

Could you sink these below the summary+description? Those fields should generally always be at the top.

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

269–271

Can you do something like:

280

Please use camelCase for variable names. Same below.

284

Do you need this temporary variable? Can you just pass Table inline?

286

You could try llvm.localescape which is variadic IIRC. There are a few others as well.

286–287

The return op.emitOpError still looks unresolved?

312–313

[electriclilies]

comments

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

284

Yup, it is necessary unfortunately, it won't convert implicilty

286–287

whoops, misread the suggestion. should be resolved now

[electriclilies]

add variadic test + remove variadic check since it's unneeded

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

286

Thanks! I added a test for localescope, and it seems like things are working fine for variadic localescope, so I'm just going to remove the check. I don't think it's possible to trigger it.

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", [Pure]> {
+  let arguments = (ins StrAttr:$intrin, Variadic<LLVM_Type>:$args);
+  let results = (outs Variadic<LLVM_Type>:$results);
+  let summary = [{Calls an intrinsic.}];
+  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 llvmBuilder = [{
+    llvm::Module *module = builder.GetInsertBlock()->getModule();
+    llvm::Intrinsic::ID id = op.getIntrinsicEnum();
+    if (!id) {
+      // If id is not valid, emit an error and don't do anything else.
+      op.emitOpError(llvm::Twine("couldn't find intrinsic \"")
+                                     + op.getIntrinAttr().str() + "\"");
+      // return success so that the error gets propagated correctly
+      return success();
+    }
+    llvm::Function *fn = nullptr;
+    if (llvm::Intrinsic::isOverloaded(id)) {
+      auto fnOrFailure = getOverloadedDeclaration(opInst, op, id, module, moduleTranslation);
+      if (failed(fnOrFailure))
+        // return success so that the error gets propagated correctly
+        return success();
+      fn = fnOrFailure.value();
+    } else {
+      fn = llvm::Intrinsic::getDeclaration(module, id, {});
+    }
+
+    auto *inst = builder.CreateCall(fn,
+                  moduleTranslation.lookupValues(opInst.getOperands()));
+    if (opInst.getNumResults() == 1)
+        moduleTranslation.mapValue(op->getResults().front())  = inst;
+
+  }];
+  let assemblyFormat = [{
+    $intrin `(` $args `)` `:` functional-type($args, $results) attr-dict
+  }];
+
+  let extraClassDeclaration = [{
+    llvm::Intrinsic::ID getIntrinsicEnum() {
+      return llvm::Function::lookupIntrinsicID(getIntrinAttr());
+    }
+  }];
+}
+
 //
 // 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/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;
 }
 
+static FailureOr<llvm::Function *>
+getOverloadedDeclaration(Operation &opInst, CallIntrinsicOp &op, llvm::Intrinsic::ID id,
+                         llvm::Module *module,
+                         LLVM::ModuleTranslation &moduleTranslation) {
+  SmallVector<llvm::Type *, 8> allArgTys;
+  for (size_t i = 0, e = opInst.getNumOperands(); i != e; ++i)
+    allArgTys.push_back(
+        moduleTranslation.convertType(opInst.getOperand(i).getType()));
+
+  llvm::Type *resTy;
+  if (opInst.getNumResults() == 0)
+    resTy = llvm::Type::getVoidTy(module->getContext());
+  else
+    resTy = moduleTranslation.convertType(opInst.getResult(0).getType());
+
+  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) {
+    op.emitOpError("Intrinsic type is not a match!");
+    return failure();
+  }
+
+  if (llvm::Intrinsic::matchIntrinsicVarArg(FT->isVarArg(), TableRef)) {
+    op.emitOpError("Intrinsic variadic-ness is not a match!");
+    return failure();
+  }
+
+  ArrayRef<llvm::Type *> OverloadedArgTys = overloadedArgTys;
+  return llvm::Intrinsic::getDeclaration(module, id, OverloadedArgTys);
+}
+
 static LogicalResult
 convertOperationImpl(Operation &opInst, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
 
   llvm::IRBuilder<>::FastMathFlagGuard fmfGuard(builder);
   if (auto fmf = dyn_cast<FastmathFlagsInterface>(opInst))
     builder.setFastMathFlags(getFastmathFlags(fmf));
 
 #include "mlir/Dialect/LLVMIR/LLVMConversions.inc"
 #include "mlir/Dialect/LLVMIR/LLVMIntrinsicConversions.inc"
 
   // Emit function calls.  If the "callee" attribute is present, this is a
   // direct function call and we also need to look up the remapped function
   // itself.  Otherwise, this is an indirect call and the callee is the first
-  // operand, look it up as a normal value.  Return the llvm::Value representing
-  // the function result, which may be of llvm::VoidTy type.
+  // operand, look it up as a normal value.  Return the llvm::Value
+  // representing the function result, which may be of llvm::VoidTy type.
   auto convertCall = [&](Operation &op) -> llvm::Value * {
     auto operands = moduleTranslation.lookupValues(op.getOperands());
     ArrayRef<llvm::Value *> operandsRef(operands);
     if (auto attr = op.getAttrOfType<FlatSymbolRefAttr>("callee"))
       return builder.CreateCall(
           moduleTranslation.lookupFunction(attr.getValue()), operandsRef);
     auto calleeType =
         op.getOperands().front().getType().cast<LLVMPointerType>();
for (llvm::Value *operand :
       // All operands should be constant - checked by verifier
       if (auto *constOperand = dyn_cast<llvm::Constant>(operand))
         lpi->addClause(constOperand);
     }
     moduleTranslation.mapValue(lpOp.getResult(), lpi);
     return success();
   }
 
-  // Emit branches.  We need to look up the remapped blocks and ignore the block
-  // arguments that were transformed into PHI nodes.
+  // Emit branches.  We need to look up the remapped blocks and ignore the
+  // block arguments that were transformed into PHI nodes.
   if (auto brOp = dyn_cast<LLVM::BrOp>(opInst)) {
     llvm::BranchInst *branch =
         builder.CreateBr(moduleTranslation.lookupBlock(brOp.getSuccessor()));
     moduleTranslation.mapBranch(&opInst, branch);
     setLoopMetadata(opInst, *branch, builder, moduleTranslation);
     return success();
   }
   if (auto condbrOp = dyn_cast<LLVM::CondBrOp>(opInst)) {

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

+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %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 <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:  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> {}
+    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
+}
+
+// -----
+
+llvm.func @error() {
+    // expected-error@+1 {{'llvm.call_intrinsic' op couldn't find intrinsic "llvm.does_not_exist"}}
+    // expected-error@+1 {{LLVM Translation failed for operation: llvm.call_intrinsic}}
+    llvm.call_intrinsic "llvm.does_not_exist"() : () -> ()
+    llvm.return
+}