[flang-commits] [flang] [llvm] [flang][OMPIRBuilder] Keep debug location in sync with insert point. (PR #89953)
Fangrui Song via flang-commits
flang-commits at lists.llvm.org
Tue May 7 09:16:11 PDT 2024
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/89953
>From 775794e398d17fead679b964e2a9bda890864b01 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 24 Apr 2024 15:28:47 +0100
Subject: [PATCH 1/2] [flang][OMPIRBuilder] Keep debug location in sync.
A customer reported an issue which I have reduced to the test in the
PR. If built with debug info enabled, the build fails with the following
error in the verifier.
!dbg attachment points at wrong subprogram for function
The problem happened because some of the functions in OMPIRBuilder.cpp
updated the insertion point with the passed in location but did not
change the current debug location. This caused a stale debug location to
be attached to the instruction.
I have solved it by replacing restoreIP with updateToLocation which
updates both the insertion point and debug location. The
updateToLocation is used in many places already, so this PR
brings functions that I have changed in line with rest of the file.
Slight issue is that I am not checking the return type of
updateToLocation as there is no good value I could return in that case.
But if we have a condition where updateToLocation will return false,
these functions will fail in any case.
I have added a test that checks that build does not fail.
---
flang/test/Integration/debug-loc-1.f90 | 27 +++++++++++++++++++++++
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 12 +++++-----
2 files changed, 33 insertions(+), 6 deletions(-)
create mode 100644 flang/test/Integration/debug-loc-1.f90
diff --git a/flang/test/Integration/debug-loc-1.f90 b/flang/test/Integration/debug-loc-1.f90
new file mode 100644
index 00000000000000..d3ae4c4ca317d8
--- /dev/null
+++ b/flang/test/Integration/debug-loc-1.f90
@@ -0,0 +1,27 @@
+!RUN: %flang_fc1 -emit-llvm -debug-info-kind=line-tables-only -fopenmp %s -o -
+
+! Test that this file builds without an error.
+
+module debugloc
+contains
+subroutine test1
+implicit none
+ integer :: i
+ real, save :: var
+
+!$omp parallel do
+do i=1,100
+ var = var + 0.1
+end do
+!$omp end parallel do
+
+end subroutine test1
+
+subroutine test2
+
+real, save :: tp
+!$omp threadprivate (tp)
+
+end subroutine test2
+
+end module debugloc
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 9f0c07ef0da910..42ea20919a5e16 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4401,7 +4401,7 @@ CallInst *OpenMPIRBuilder::createOMPAlloc(const LocationDescription &Loc,
Value *Size, Value *Allocator,
std::string Name) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
@@ -4418,7 +4418,7 @@ CallInst *OpenMPIRBuilder::createOMPFree(const LocationDescription &Loc,
Value *Addr, Value *Allocator,
std::string Name) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
@@ -4434,7 +4434,7 @@ CallInst *OpenMPIRBuilder::createOMPInteropInit(
omp::OMPInteropType InteropType, Value *Device, Value *NumDependences,
Value *DependenceAddress, bool HaveNowaitClause) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
@@ -4462,7 +4462,7 @@ CallInst *OpenMPIRBuilder::createOMPInteropDestroy(
const LocationDescription &Loc, Value *InteropVar, Value *Device,
Value *NumDependences, Value *DependenceAddress, bool HaveNowaitClause) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
@@ -4491,7 +4491,7 @@ CallInst *OpenMPIRBuilder::createOMPInteropUse(const LocationDescription &Loc,
Value *DependenceAddress,
bool HaveNowaitClause) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
Value *Ident = getOrCreateIdent(SrcLocStr, SrcLocStrSize);
@@ -4517,7 +4517,7 @@ CallInst *OpenMPIRBuilder::createCachedThreadPrivate(
const LocationDescription &Loc, llvm::Value *Pointer,
llvm::ConstantInt *Size, const llvm::Twine &Name) {
IRBuilder<>::InsertPointGuard IPG(Builder);
- Builder.restoreIP(Loc.IP);
+ updateToLocation(Loc);
uint32_t SrcLocStrSize;
Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
>From 4b2122f671bf29e7a1d6d285fc31040774967399 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 3 May 2024 15:23:04 +0100
Subject: [PATCH 2/2] Address review comment.
I have added CHECK lines as was suggested in the review.
---
flang/test/Integration/debug-loc-1.f90 | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/flang/test/Integration/debug-loc-1.f90 b/flang/test/Integration/debug-loc-1.f90
index d3ae4c4ca317d8..5fe2c8e31dd9d1 100644
--- a/flang/test/Integration/debug-loc-1.f90
+++ b/flang/test/Integration/debug-loc-1.f90
@@ -1,4 +1,4 @@
-!RUN: %flang_fc1 -emit-llvm -debug-info-kind=line-tables-only -fopenmp %s -o -
+!RUN: %flang_fc1 -emit-llvm -debug-info-kind=line-tables-only -fopenmp %s -o - | FileCheck %s
! Test that this file builds without an error.
@@ -9,6 +9,7 @@ subroutine test1
integer :: i
real, save :: var
+! CHECK: DILocation(line: [[@LINE+1]], {{.*}})
!$omp parallel do
do i=1,100
var = var + 0.1
@@ -21,6 +22,8 @@ subroutine test2
real, save :: tp
!$omp threadprivate (tp)
+! CHECK: DILocation(line: [[@LINE+1]], {{.*}})
+ tp = tp + 1
end subroutine test2
More information about the flang-commits
mailing list