Skip to content

[MLIR] Model llvm.inline_asm side_effects #91507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2024

Conversation

ThomasRaoux
Copy link
Contributor

Allow more cleanups on inline_asm ops modeling side effects based on the side_effect attributed.

Allow more cleanups on inline_asm ops modeling side effects based
on the side_effect attributed.
@llvmbot
Copy link
Member

llvmbot commented May 8, 2024

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Thomas Raoux (ThomasRaoux)

Changes

Allow more cleanups on inline_asm ops modeling side effects based on the side_effect attributed.


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

3 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+1-1)
  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+13)
  • (modified) mlir/test/Dialect/LLVMIR/canonicalize.mlir (+11)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 6655ce6f123e1..4b91708ea1aa7 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1795,7 +1795,7 @@ def LLVM_FenceOp : LLVM_Op<"fence">, LLVM_MemOpPatterns {
   let hasVerifier = 1;
 }
 
-def LLVM_InlineAsmOp : LLVM_Op<"inline_asm", []> {
+def LLVM_InlineAsmOp : LLVM_Op<"inline_asm", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let description = [{
     The InlineAsmOp mirrors the underlying LLVM semantics with a notable
     exception: the embedded `asm_string` is not allowed to define or reference
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 7be493d5992c4..7d33d05feb650 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3034,6 +3034,19 @@ LogicalResult LinkerOptionsOp::verify() {
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// InlineAsmOp
+//===----------------------------------------------------------------------===//
+
+void InlineAsmOp::getEffects(
+    SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>>
+        &effects) {
+  if (getHasSideEffects()) {
+    effects.emplace_back(MemoryEffects::Write::get());
+    effects.emplace_back(MemoryEffects::Read::get());
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // LLVMDialect initialization, type parsing, and registration.
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/LLVMIR/canonicalize.mlir b/mlir/test/Dialect/LLVMIR/canonicalize.mlir
index 6b265bbbdbfb2..15f960167cb5f 100644
--- a/mlir/test/Dialect/LLVMIR/canonicalize.mlir
+++ b/mlir/test/Dialect/LLVMIR/canonicalize.mlir
@@ -248,3 +248,14 @@ llvm.func @volatile_load(%x : !llvm.ptr) {
   %3 = llvm.load %x  atomic unordered { alignment = 1 } : !llvm.ptr -> i8
   llvm.return
 }
+
+// -----
+
+// CHECK-LABEL: func @inline_asm_side_effects
+llvm.func @inline_asm_side_effects(%x : i32) {
+  // CHECK-NOT: llvm.inline_asm "pure inline asm"
+  llvm.inline_asm "pure inline asm", "r" %x : (i32) -> ()
+  // CHECK: llvm.inline_asm has_side_effects "inline asm with side effects"
+  llvm.inline_asm has_side_effects "inline asm with side effects", "r" %x : (i32) -> ()
+  llvm.return
+}

@ThomasRaoux ThomasRaoux requested a review from antiagainst May 8, 2024 17:27
@ThomasRaoux ThomasRaoux merged commit 5526c8a into llvm:main May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants