[mlir][LLVM] Allow call_intrinsic to inline

[nicolasvasilache]

LLVM::CallIntrinsicOp was previously overlooked from the isLegalToInline.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Nicolas Vasilache (nicolasvasilache)

Changes

LLVM::CallIntrinsicOp was previously overlooked from the isLegalToInline.

---

Full diff: https://github.com/llvm/llvm-project/pull/70940.diff

2 Files Affected:

• (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp (+1)
• (modified) mlir/test/Dialect/LLVMIR/inlining.mlir (+2)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
index b40be73ff21f703..6063abdba7b9a1f 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
@@ -694,6 +694,7 @@ struct LLVMInlinerInterface : public DialectInlinerInterface {
             LLVM::AtomicRMWOp,
             LLVM::AtomicCmpXchgOp,
             LLVM::CallOp,
+            LLVM::CallIntrinsicOp,
             LLVM::DbgDeclareOp,
             LLVM::DbgLabelOp,
             LLVM::DbgValueOp,
diff --git a/mlir/test/Dialect/LLVMIR/inlining.mlir b/mlir/test/Dialect/LLVMIR/inlining.mlir
index 3f14dc6de6b764c..1296b8e031c1330 100644
--- a/mlir/test/Dialect/LLVMIR/inlining.mlir
+++ b/mlir/test/Dialect/LLVMIR/inlining.mlir
@@ -28,6 +28,7 @@ func.func @inner_func_inlinable(%ptr : !llvm.ptr) -> i32 {
   llvm.unreachable
 ^bb2:
   llvm.intr.stackrestore %stack : !llvm.ptr
+  llvm.call_intrinsic "llvm.x86.sse41.round.ss"() : () -> (vector<8xf32>)
   return %1 : i32
 }
 
@@ -50,6 +51,7 @@ func.func @inner_func_inlinable(%ptr : !llvm.ptr) -> i32 {
 // CHECK: llvm.inline_asm has_side_effects "foo", "bar"
 // CHECK: llvm.unreachable
 // CHECK: llvm.intr.stackrestore %[[STACK]]
+// CHECK: llvm.call_intrinsic "llvm.x86.sse41.round.ss"(
 func.func @test_inline(%ptr : !llvm.ptr) -> i32 {
   %0 = call @inner_func_inlinable(%ptr) : (!llvm.ptr) -> i32
   return %0 : i32

[definelicht]

LGTM! We can think about whether it's time to remove the allowlist, if there are few or no unhandled cases left (potentially making a disallowlist instead - debug info handling comes to mind).

[joker-eph]

I definitely don't quite get why we have an allowlist in the first place: does the LLVM inliner has such list?

[definelicht]

I definitely don't quite get why we have an allowlist in the first place: does the LLVM inliner has such list?

It is/was a migration strategy to be able to gradually develop the inliner to be feature complete. The LLVM inliner is quite, let's say, "decentralized", so there's no single place to see a list of features that must be supported :-)