Skip to content

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented May 31, 2024

Fixes a bug uncovered by pr43337.f90 in the test suite.

In particular, this emits argNo debug info only if the parent op of a block is a func.func op. This avoids DI conflicts when a function contains a nested OpenMP region that itself has block arguments with DI attached to them; for example, omp.parallel with delayed privatization enabled.

Fixes a bug uncovered by (pr43337.f90)[https://github.com/llvm/llvm-test-suite/blob/main/Fortran/gfortran/regression/gomp/pr43337.f90] in the test suite.

In particular, this emits `argNo` debug info only if the parent op of a
block is a `func.func` op. This avoids DI conflicts when a function
contains a nested OpenMP region that itself has block arguments with DI
attached to them; for example, `omp.parallel` with delayed privatization
enabled.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels May 31, 2024
@llvmbot
Copy link
Member

llvmbot commented May 31, 2024

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Kareem Ergawy (ergawy)

Changes

Fixes a bug uncovered by (pr43337.f90)[https://github.com/llvm/llvm-test-suite/blob/main/Fortran/gfortran/regression/gomp/pr43337.f90] in the test suite.

In particular, this emits argNo debug info only if the parent op of a block is a func.func op. This avoids DI conflicts when a function contains a nested OpenMP region that itself has block arguments with DI attached to them; for example, omp.parallel with delayed privatization enabled.


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

2 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/AddDebugInfo.cpp (+3-3)
  • (added) flang/test/Lower/OpenMP/debug_info_conflict.f90 (+16)
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index fb7c0bf0d1f97..e7a3d93a533ef 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -94,10 +94,10 @@ void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
   // DeclareOp is generated. In that case, DeclareOp may point to an
   // intermediate op and not to BlockArgument. We need to find those cases and
   // walk the chain to get to the actual argument.
-
   unsigned argNo = 0;
-  if (auto Arg = llvm::dyn_cast<mlir::BlockArgument>(declOp.getMemref()))
-    argNo = Arg.getArgNumber() + 1;
+  if (mlir::isa<mlir::func::FuncOp>(declOp->getParentOp()))
+    if (auto arg = llvm::dyn_cast<mlir::BlockArgument>(declOp.getMemref()))
+      argNo = arg.getArgNumber() + 1;
 
   auto tyAttr = typeGen.convertType(fir::unwrapRefType(declOp.getType()),
                                     fileAttr, scopeAttr, declOp.getLoc());
diff --git a/flang/test/Lower/OpenMP/debug_info_conflict.f90 b/flang/test/Lower/OpenMP/debug_info_conflict.f90
new file mode 100644
index 0000000000000..5e52db281da23
--- /dev/null
+++ b/flang/test/Lower/OpenMP/debug_info_conflict.f90
@@ -0,0 +1,16 @@
+! Tests that there no debug-info conflicts arise because of DI attached to nested
+! OMP regions arguments.
+
+! RUN: %flang -c -fopenmp -g -mmlir --openmp-enable-delayed-privatization=true \
+! RUN:   %s -o - 2>&1 | FileCheck %s
+
+subroutine bar (b)
+  integer :: a, b
+!$omp parallel
+  do a = 1, 10
+    b = a
+  end do
+!$omp end parallel
+end subroutine bar
+
+! CHECK-NOT: conflicting debug info for argument

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the bug fix

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ergawy ergawy merged commit 5bfc444 into llvm:main Jun 3, 2024
! OMP regions arguments.

! RUN: %flang -c -fopenmp -g -mmlir --openmp-enable-delayed-privatization=true \
! RUN: %s -o - 2>&1 | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think using %flang_fc1 -emit-llvm will also reproduce this problem and it will be more in line with the other tests in this folder instead of using %flang. You can use -debug-info-kind=standalone for -g.

Copy link
Contributor

@abidh abidh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. Looks good to me apart from the small nit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants