[Mlir-commits] [mlir] [mlir][llvmir][debug] Correctly generate location for phi nodes. (PR #105534)
Abid Qadeer
llvmlistbot at llvm.org
Wed Aug 21 14:27:08 PDT 2024
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/105534
>From d5adacf0668bcdfea63c1afca763dca6f079783a Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Thu, 11 Jul 2024 20:33:23 +0100
Subject: [PATCH 1/3] [mlir][llvmir][debug] Correctly generate location for phi
nodes.
In convertBlockImpl, the debug location is set on the builder before
the op is processed. This results in correct location being given to
corresponding llvm instructions. But same is not done when phi nodes
are created a few lines above. This result is phi nodes getting whatever
the current debug location of the builder is. It can be nothing or in
worst case a stale location. Fixed by calling SetCurrentDebugLocation
before generating phi nodes.
---
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 2 +
mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir | 47 ++++++++++++++++++++
2 files changed, 49 insertions(+)
create mode 100644 mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 930300d26c4479..2827713e2bf213 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -947,6 +947,8 @@ LogicalResult ModuleTranslation::convertBlockImpl(Block &bb,
if (!isCompatibleType(wrappedType))
return emitError(bb.front().getLoc(),
"block argument does not have an LLVM type");
+ builder.SetCurrentDebugLocation(
+ debugTranslation->translateLoc(arg.getLoc(), subprogram));
llvm::Type *type = convertType(wrappedType);
llvm::PHINode *phi = builder.CreatePHI(type, numPredecessors);
mapValue(arg, phi);
diff --git a/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
new file mode 100644
index 00000000000000..809a0a33fe7cb3
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
@@ -0,0 +1,47 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+
+module attributes {} {
+ llvm.func @test(%arg0: !llvm.ptr) {
+ %0 = llvm.mlir.constant(1 : i64) : i64 loc(#loc2)
+ %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr loc(#loc2)
+ %3 = llvm.mlir.constant(100 : index) : i64 loc(#loc2)
+ %7 = llvm.trunc %0 : i64 to i32 loc(#loc2)
+ llvm.br ^bb1(%7, %3 : i32, i64) loc(#loc2)
+ ^bb1(%8: i32 loc(#loc4), %9: i64 loc(#loc5)): // 2 preds: ^bb0, ^bb2
+ %10 = llvm.icmp "sgt" %9, %0 : i64 loc(#loc3)
+ llvm.cond_br %10, ^bb2, ^bb3 loc(#loc3)
+ ^bb2: // pred: ^bb1
+ %13 = llvm.load %1 : !llvm.ptr -> i32 loc(#loc3)
+ %14 = llvm.add %13, %7 : i32 loc(#loc3)
+ %15 = llvm.sub %9, %0 : i64 loc(#loc3)
+ llvm.br ^bb1(%14, %15 : i32, i64) loc(#loc3)
+ ^bb3: // pred: ^bb1
+ llvm.return loc(#loc3)
+ } loc(#loc6)
+}
+
+#int_ty = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "integer",
+ sizeInBits = 32, encoding = DW_ATE_signed>
+#file = #llvm.di_file<"test.f90" in "">
+#cu = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+ emissionKind = Full>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal,
+ types = #int_ty>
+#sp = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #cu, scope = #file,
+ name = "test", file = #file, line = 1, scopeLine = 1,
+ subprogramFlags = Definition, type = #sp_ty>
+
+#loc1 = loc("test.f90":1:1)
+#loc2 = loc("test.f90":15:22)
+#loc3 = loc("test.f90":26:3)
+#loc4 = loc("test.f90":8:2)
+#loc5 = loc("test.f90":9:5)
+#loc6 = loc(fused<#sp>[#loc1])
+
+// CHECK-LABEl: define void @test
+// CHECK: phi i32{{.*}}!dbg ![[LOC1:[0-9]+]]
+// CHECK: phi i64{{.*}}!dbg ![[LOC2:[0-9]+]]
+// CHECK: ![[LOC1]] = !DILocation(line: 8, column: 2{{.*}})
+// CHECK: ![[LOC2]] = !DILocation(line: 9, column: 5{{.*}})
>From 7ab075a4f129e1b35db3b5d5e888d237b98294c3 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 21 Aug 2024 17:32:46 +0100
Subject: [PATCH 2/3] Handle review comments.
Remove the things from the test case which were not really essential for the test.
---
mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir | 51 ++++++++-------------
1 file changed, 18 insertions(+), 33 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
index 809a0a33fe7cb3..f494dc57bbad6b 100644
--- a/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
@@ -1,46 +1,31 @@
// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+llvm.func @test_phi_locations(%arg0: !llvm.ptr) {
+ %0 = llvm.mlir.constant(1 : i64) : i64 loc(#loc1)
+ %1 = llvm.mlir.constant(100 : i32) : i32 loc(#loc1)
+ llvm.br ^bb1(%1, %0 : i32, i64) loc(#loc1)
+^bb1(%2: i32 loc(#loc2), %3: i64 loc(#loc3)):
+ %4 = llvm.icmp "sgt" %3, %0 : i64 loc(#loc1)
+ llvm.cond_br %4, ^bb2, ^bb1(%2, %3 : i32, i64) loc(#loc1)
+^bb2:
+ llvm.return loc(#loc1)
+} loc(#loc4)
-module attributes {} {
- llvm.func @test(%arg0: !llvm.ptr) {
- %0 = llvm.mlir.constant(1 : i64) : i64 loc(#loc2)
- %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr loc(#loc2)
- %3 = llvm.mlir.constant(100 : index) : i64 loc(#loc2)
- %7 = llvm.trunc %0 : i64 to i32 loc(#loc2)
- llvm.br ^bb1(%7, %3 : i32, i64) loc(#loc2)
- ^bb1(%8: i32 loc(#loc4), %9: i64 loc(#loc5)): // 2 preds: ^bb0, ^bb2
- %10 = llvm.icmp "sgt" %9, %0 : i64 loc(#loc3)
- llvm.cond_br %10, ^bb2, ^bb3 loc(#loc3)
- ^bb2: // pred: ^bb1
- %13 = llvm.load %1 : !llvm.ptr -> i32 loc(#loc3)
- %14 = llvm.add %13, %7 : i32 loc(#loc3)
- %15 = llvm.sub %9, %0 : i64 loc(#loc3)
- llvm.br ^bb1(%14, %15 : i32, i64) loc(#loc3)
- ^bb3: // pred: ^bb1
- llvm.return loc(#loc3)
- } loc(#loc6)
-}
-
-#int_ty = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "integer",
- sizeInBits = 32, encoding = DW_ATE_signed>
#file = #llvm.di_file<"test.f90" in "">
#cu = #llvm.di_compile_unit<id = distinct[0]<>,
sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
emissionKind = Full>
-#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal,
- types = #int_ty>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
#sp = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #cu, scope = #file,
- name = "test", file = #file, line = 1, scopeLine = 1,
- subprogramFlags = Definition, type = #sp_ty>
+ name = "test_phi_locations", file = #file, subprogramFlags = Definition,
+ type = #sp_ty>
-#loc1 = loc("test.f90":1:1)
-#loc2 = loc("test.f90":15:22)
-#loc3 = loc("test.f90":26:3)
-#loc4 = loc("test.f90":8:2)
-#loc5 = loc("test.f90":9:5)
-#loc6 = loc(fused<#sp>[#loc1])
+#loc1 = loc("test.f90":15:22)
+#loc2 = loc("test.f90":8:2)
+#loc3 = loc("test.f90":9:5)
+#loc4 = loc(fused<#sp>[#loc1])
-// CHECK-LABEl: define void @test
+// CHECK-LABEL: define void @test
// CHECK: phi i32{{.*}}!dbg ![[LOC1:[0-9]+]]
// CHECK: phi i64{{.*}}!dbg ![[LOC2:[0-9]+]]
// CHECK: ![[LOC1]] = !DILocation(line: 8, column: 2{{.*}})
>From 7216fae4394d1236ab407a2f2938a87a41327e09 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 21 Aug 2024 22:25:30 +0100
Subject: [PATCH 3/3] Handle review comments.
Use the correct name of the function in CHECK.
---
mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
index f494dc57bbad6b..fd045026052848 100644
--- a/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-phi-loc.mlir
@@ -25,7 +25,7 @@ llvm.func @test_phi_locations(%arg0: !llvm.ptr) {
#loc3 = loc("test.f90":9:5)
#loc4 = loc(fused<#sp>[#loc1])
-// CHECK-LABEL: define void @test
+// CHECK-LABEL: define void @test_phi_locations
// CHECK: phi i32{{.*}}!dbg ![[LOC1:[0-9]+]]
// CHECK: phi i64{{.*}}!dbg ![[LOC2:[0-9]+]]
// CHECK: ![[LOC1]] = !DILocation(line: 8, column: 2{{.*}})
More information about the Mlir-commits
mailing list