[MLIR][LLVMIR] Import: fix llvm.call attribute inheritance

[bcardosolopes]

inst->getFnAttr(Kind) fallbacks to check if the parent has an attribute, which breaks roundtriping the LLVM IR. This change actually checks only in the call attribute list (no fallback to parent queries).

It's possible to argue that this small optimization isn't harmful, but seems too early if it's breaking roundtrip behavior.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Bruno Cardoso Lopes (bcardosolopes)

Changes

…ributes

inst->getFnAttr(Kind) fallbacks to check if the parent has an attribute, which breaks roundtriping the LLVM IR. This change actually checks only in the call attribute list (no fallback to parent queries).

It's possible to argue that this small optimization isn't harmful, but seems too early if it's breaking roundtrip behavior.

---

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

2 Files Affected:

• (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+8-3)
• (added) mlir/test/Target/LLVMIR/Import/call-attributes.ll (+18)

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 }

[xlauko]

lgtm, except too long PR name

[gysit]

Thanks makes sense!

LGTM

It could make sense to add the test to call-argument-attributes.ll?