[Mlir-commits] [mlir] [MLIR][LLVM] Fix debug value/declare import in face of landing pads (PR #132871)

[Bruno Cardoso Lopes]

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

>From 6874eda43196e3a508e7958b9376e37730bab0ec Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Mon, 24 Mar 2025 20:40:30 -0700
Subject: [PATCH 1/2] [MLIR][LLVM] Fix debug value/declare import in face of
 landing pads

Debug value/declare operations imported before landing pad operations at the bb
start break invoke op verification:

```
error: first operation in unwind destination should be a llvm.landingpad operation
```

This this issue by making the placement slightly more smart.
---
 mlir/lib/Target/LLVMIR/ModuleImport.cpp     | 17 +++++++-
 mlir/test/Target/LLVMIR/Import/exception.ll | 44 ++++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 7657661fa7b74..a9ed256816436 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -2512,7 +2512,22 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
     Block *dominatedBlock = (*dominatedBlocks.begin())->getBlock();
     builder.setInsertionPoint(dominatedBlock->getTerminator());
   } else {
-    builder.setInsertionPointAfterValue(*argOperand);
+    Value insertPt = *argOperand;
+    if (!op) {
+      // The value might be coming from a phi value and is now a block argument,
+      // which means the insertion point is set to the start of the block. If
+      // this block is a target destination of an invoke, the insertion point
+      // must happen after the landing pad operation.
+      auto blockArg = llvm::cast<BlockArgument>(*argOperand);
+      mlir::Block *insertionBlock = blockArg.getOwner();
+      if (!insertionBlock->empty() &&
+          isa<LandingpadOp>(insertionBlock->front())) {
+        auto landingPad = cast<LandingpadOp>(insertionBlock->front());
+        insertPt = landingPad.getRes();
+      }
+    }
+
+    builder.setInsertionPointAfterValue(insertPt);
   }
   auto locationExprAttr =
       debugImporter->translateExpression(dbgIntr->getExpression());
diff --git a/mlir/test/Target/LLVMIR/Import/exception.ll b/mlir/test/Target/LLVMIR/Import/exception.ll
index 440d89ec147f7..1451104920623 100644
--- a/mlir/test/Target/LLVMIR/Import/exception.ll
+++ b/mlir/test/Target/LLVMIR/Import/exception.ll
@@ -1,4 +1,4 @@
-; RUN: mlir-translate -import-llvm %s | FileCheck %s
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
 @_ZTIi = external dso_local constant ptr
 @_ZTIii= external dso_local constant ptr
@@ -148,3 +148,45 @@ exit:
   ; CHECK:    llvm.return
   ret void
 }
+
+; // -----
+
+declare i32 @__gxx_personality_v0(...)
+declare void @foo(ptr)
+
+; CHECK-LABEL: @invokeLandingpad
+define i32 @invokeLandingpad(ptr %addr) personality ptr @__gxx_personality_v0 {
+entry:
+  %1 = alloca i8
+  %zzz = load i32, ptr %addr, align 4
+  invoke void @foo(ptr %1) to label %bb1 unwind label %bb2
+
+bb1:
+  %yyy = load i32, ptr %addr, align 4
+  invoke void @foo(ptr %1) to label %bb3 unwind label %bb2
+
+; CHECK: ^bb{{.*}}(%[[ARG:.*]]: i32):
+bb2:
+  %phi_var = phi i32 [ %zzz, %entry ], [ %yyy, %bb1 ], !dbg !5
+  ; CHECK: llvm.landingpad cleanup : !llvm.struct<(ptr, i32)>
+  ; CHECK-NEXT: llvm.intr.dbg.value #di_local_variable #llvm.di_expression<[DW_OP_LLVM_fragment(64, 64)]> = %[[ARG]] : i32
+  %3 = landingpad { ptr, i32 } cleanup, !dbg !5
+  #dbg_value(i32 %phi_var, !8, !DIExpression(DW_OP_LLVM_fragment, 64, 64), !7)
+  br label %bb3
+
+bb3:
+  ; CHECK: llvm.return %{{[0-9]+}} : i32
+  ret i32 1
+}
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "landingpad.ll", directory: "/")
+!3 = distinct !DISubprogram(name: "instruction_loc", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!4 = distinct !DISubprogram(name: "callee", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!5 = !DILocation(line: 1, column: 2, scope: !3)
+!6 = !DILocation(line: 2, column: 2, scope: !3)
+!7 = !DILocation(line: 7, column: 4, scope: !4, inlinedAt: !6)
+!8 = !DILocalVariable(scope: !4, name: "size")

>From cd39dbcf1876f316a153a447e19b9cb2d4929faf Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Tue, 25 Mar 2025 10:57:33 -0700
Subject: [PATCH 2/2] Address reviews from dinistro and gysit

---
 mlir/lib/Target/LLVMIR/ModuleImport.cpp | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index a9ed256816436..00b16c86157e9 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -2513,18 +2513,15 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
     builder.setInsertionPoint(dominatedBlock->getTerminator());
   } else {
     Value insertPt = *argOperand;
-    if (!op) {
-      // The value might be coming from a phi value and is now a block argument,
+    if (auto blockArg = dyn_cast<BlockArgument>(*argOperand)) {
+      // The value might be coming from a phi node and is now a block argument,
       // which means the insertion point is set to the start of the block. If
       // this block is a target destination of an invoke, the insertion point
       // must happen after the landing pad operation.
-      auto blockArg = llvm::cast<BlockArgument>(*argOperand);
-      mlir::Block *insertionBlock = blockArg.getOwner();
+      Block *insertionBlock = argOperand->getParentBlock();
       if (!insertionBlock->empty() &&
-          isa<LandingpadOp>(insertionBlock->front())) {
-        auto landingPad = cast<LandingpadOp>(insertionBlock->front());
-        insertPt = landingPad.getRes();
-      }
+          isa<LandingpadOp>(insertionBlock->front()))
+        insertPt = cast<LandingpadOp>(insertionBlock->front()).getRes();
     }
 
     builder.setInsertionPointAfterValue(insertPt);