[Mlir-commits] [mlir] [MLIR][LLVMIR] Import: fix llvm.call attribute inheritance (PR #130221)

[Bruno Cardoso Lopes]

https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/130221

>From 17717c49aea4423312e1c19b3eec3f391a192bfd Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Thu, 6 Mar 2025 17:39:24 -0800
Subject: [PATCH 1/2] [MLIR][LLVMIR] Import: fix function attribute inheritance
 on call attributes

Before this PR `inst->getFnAttr(Kind)` fallbacks to check if the parent has an
attribute, which breaks roundtriping the LLVM IR. It's possible to argue that
this small optimization isn't harmful, but seems too early if it's breaking
roundtrip behavior.
---
 mlir/lib/Target/LLVMIR/ModuleImport.cpp        | 11 ++++++++---
 .../Target/LLVMIR/Import/call-attributes.ll    | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 mlir/test/Target/LLVMIR/Import/call-attributes.ll

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index cff5a1940e77e..f24c2777cbbb8 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -2263,10 +2263,15 @@ LogicalResult ModuleImport::convertInvokeAttributes(llvm::InvokeInst *inst,
 LogicalResult ModuleImport::convertCallAttributes(llvm::CallInst *inst,
                                                   CallOp op) {
   setFastmathFlagsAttr(inst, op.getOperation());
+  // Query the attributes directly instead of using `inst->getFnAttr(Kind)`, the
+  // latter does additional lookup to the parent and inherits, changing the
+  // semantics too early.
+  llvm::AttributeList callAttrs = inst->getAttributes();
+
   op.setTailCallKind(convertTailCallKindFromLLVM(inst->getTailCallKind()));
-  op.setConvergent(inst->isConvergent());
-  op.setNoUnwind(inst->doesNotThrow());
-  op.setWillReturn(inst->hasFnAttr(llvm::Attribute::WillReturn));
+  op.setConvergent(callAttrs.getFnAttr(llvm::Attribute::Convergent).isValid());
+  op.setNoUnwind(callAttrs.getFnAttr(llvm::Attribute::NoUnwind).isValid());
+  op.setWillReturn(callAttrs.getFnAttr(llvm::Attribute::WillReturn).isValid());
 
   llvm::MemoryEffects memEffects = inst->getMemoryEffects();
   ModRefInfo othermem = convertModRefInfoFromLLVM(
diff --git a/mlir/test/Target/LLVMIR/Import/call-attributes.ll b/mlir/test/Target/LLVMIR/Import/call-attributes.ll
new file mode 100644
index 0000000000000..a1414de49c41d
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/call-attributes.ll
@@ -0,0 +1,18 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
+
+%struct.S = type { i8 }
+
+; CHECK-LABEL: @t
+define void @t(i1 %0) #0 {
+  %3 = alloca %struct.S, align 1
+  ; CHECK-NOT: llvm.call @z(%1) {no_unwind} : (!llvm.ptr) -> ()
+  ; CHECK: llvm.call @z(%1) : (!llvm.ptr) -> ()
+  call void @z(ptr %3)
+  ret void
+}
+
+define linkonce_odr void @z(ptr %0) #0 {
+  ret void
+}
+
+attributes #0 = { nounwind }

>From e8107cd61869cbf40cd08a73495ae7cab5c1cf38 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Fri, 7 Mar 2025 10:02:45 -0800
Subject: [PATCH 2/2] Move test around

---
 .../LLVMIR/Import/call-argument-attributes.ll | 21 ++++++++++++++++++-
 .../Target/LLVMIR/Import/call-attributes.ll   | 18 ----------------
 2 files changed, 20 insertions(+), 19 deletions(-)
 delete mode 100644 mlir/test/Target/LLVMIR/Import/call-attributes.ll

diff --git a/mlir/test/Target/LLVMIR/Import/call-argument-attributes.ll b/mlir/test/Target/LLVMIR/Import/call-argument-attributes.ll
index fa39c79bf0859..842639df0d44b 100644
--- a/mlir/test/Target/LLVMIR/Import/call-argument-attributes.ll
+++ b/mlir/test/Target/LLVMIR/Import/call-argument-attributes.ll
@@ -1,4 +1,4 @@
-; RUN: mlir-translate -import-llvm %s | FileCheck %s
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
 ; CHECK-LABEL: llvm.func @somefunc(i32, !llvm.ptr)
 declare void @somefunc(i32, ptr)
@@ -20,3 +20,22 @@ define i16 @test_call_arg_attrs_indirect(i16 %0, ptr %1) {
   %3 = tail call signext i16 %1(i16 noundef signext %0)
   ret i16 %3
 }
+
+; // -----
+
+%struct.S = type { i8 }
+
+; CHECK-LABEL: @t
+define void @t(i1 %0) #0 {
+  %3 = alloca %struct.S, align 1
+  ; CHECK-NOT: llvm.call @z(%1) {no_unwind} : (!llvm.ptr) -> ()
+  ; CHECK: llvm.call @z(%1) : (!llvm.ptr) -> ()
+  call void @z(ptr %3)
+  ret void
+}
+
+define linkonce_odr void @z(ptr %0) #0 {
+  ret void
+}
+
+attributes #0 = { nounwind }
diff --git a/mlir/test/Target/LLVMIR/Import/call-attributes.ll b/mlir/test/Target/LLVMIR/Import/call-attributes.ll
deleted file mode 100644
index a1414de49c41d..0000000000000
--- a/mlir/test/Target/LLVMIR/Import/call-attributes.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
-
-%struct.S = type { i8 }
-
-; CHECK-LABEL: @t
-define void @t(i1 %0) #0 {
-  %3 = alloca %struct.S, align 1
-  ; CHECK-NOT: llvm.call @z(%1) {no_unwind} : (!llvm.ptr) -> ()
-  ; CHECK: llvm.call @z(%1) : (!llvm.ptr) -> ()
-  call void @z(ptr %3)
-  ret void
-}
-
-define linkonce_odr void @z(ptr %0) #0 {
-  ret void
-}
-
-attributes #0 = { nounwind }