[flang-commits] [flang] 5bfc444 - [flang] Emit `argNo` debug info only for `func` block args (#93921)

[via flang-commits]

Author: Kareem Ergawy
Date: 2024-06-03T11:33:00+02:00
New Revision: 5bfc444524d74b714b9efb2dc00c7bc36a3838e2

URL: https://github.com/llvm/llvm-project/commit/5bfc444524d74b714b9efb2dc00c7bc36a3838e2
DIFF: https://github.com/llvm/llvm-project/commit/5bfc444524d74b714b9efb2dc00c7bc36a3838e2.diff

LOG: [flang] Emit `argNo` debug info only for `func` block args (#93921)

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.

Added: 
    flang/test/Lower/OpenMP/debug_info_conflict.f90

Modified: 
    flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
    flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
    flang/lib/Optimizer/Dialect/FIROps.cpp
    flang/lib/Optimizer/Transforms/AddDebugInfo.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
index b68a39bf374bd..47b80cca5d649 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
@@ -160,6 +160,7 @@ inline mlir::NamedAttribute getAdaptToByRefAttr(Builder &builder) {
           builder.getUnitAttr()};
 }
 
+bool isDummyArgument(mlir::Value v);
 } // namespace fir
 
 #endif // FORTRAN_OPTIMIZER_DIALECT_FIROPSSUPPORT_H

diff  --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 9d0d706a85c5e..fd40386a6d817 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -28,16 +28,6 @@ using namespace mlir;
 // AliasAnalysis: alias
 //===----------------------------------------------------------------------===//
 
-static bool isDummyArgument(mlir::Value v) {
-  auto blockArg{mlir::dyn_cast<mlir::BlockArgument>(v)};
-  if (!blockArg)
-    return false;
-
-  auto *owner{blockArg.getOwner()};
-  return owner->isEntryBlock() &&
-         mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp());
-}
-
 /// Temporary function to skip through all the no op operations
 /// TODO: Generalize support of fir.load
 static mlir::Value getOriginalDef(mlir::Value v) {
@@ -85,7 +75,7 @@ bool AliasAnalysis::Source::isTargetOrPointer() const {
 
 bool AliasAnalysis::Source::isDummyArgument() const {
   if (auto v = origin.u.dyn_cast<mlir::Value>()) {
-    return ::isDummyArgument(v);
+    return fir::isDummyArgument(v);
   }
   return false;
 }

diff  --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index b541b7cdc7a5b..9672cdccc111a 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -3908,6 +3908,16 @@ std::optional<std::int64_t> fir::getIntIfConstant(mlir::Value value) {
   return {};
 }
 
+bool fir::isDummyArgument(mlir::Value v) {
+  auto blockArg{mlir::dyn_cast<mlir::BlockArgument>(v)};
+  if (!blockArg)
+    return false;
+
+  auto *owner{blockArg.getOwner()};
+  return owner->isEntryBlock() &&
+         mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp());
+}
+
 mlir::Type fir::applyPathToType(mlir::Type eleTy, mlir::ValueRange path) {
   for (auto i = path.begin(), end = path.end(); eleTy && i < end;) {
     eleTy = llvm::TypeSwitch<mlir::Type, mlir::Type>(eleTy)

diff  --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index fb7c0bf0d1f97..810ebbd3da3ee 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -94,10 +94,11 @@ 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 (fir::isDummyArgument(declOp.getMemref())) {
+    auto arg = llvm::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