Skip to content

Conversation

@abidh
Copy link
Contributor

@abidh abidh commented Feb 4, 2025

We currently drop the DeclareOp which are not in the entry block of the function. This was done to side step a problem with the variables in OpenMP target region. Now that issue has been addressed in #118314 so we can lift this restriction as well.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Feb 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2025

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

Author: Abid Qadeer (abidh)

Changes

We currently drop the DeclareOp which are not in the entry block of the function. This was done to side step a problem with the variables in OpenMP target region. I have opened #118314 to address the original issue. Once that PR is merged, we can lift this restriction as well.

So this will have to wait till #118314 is approved and merged.


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

1 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/AddDebugInfo.cpp (+1-5)
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 16404fcda57b482..ee4d14dc6ebe0cd 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -503,11 +503,7 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
   funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
 
   funcOp.walk([&](fir::cg::XDeclareOp declOp) {
-    // FIXME: We currently dont handle variables that are not in the entry
-    // blocks of the fuctions. These may be variable or arguments used in the
-    // OpenMP target regions.
-    if (&funcOp.front() == declOp->getBlock())
-      handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
+    handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
   });
   // commonBlockMap ensures that we don't create multiple DICommonBlockAttr of
   // the same name in one function. But it is ok (rather required) to create

We currently drop the `DeclareOp` which are not in the entry block
of the function. This was done to side step a problem with the
variables in OpenMP target region. I have opened llvm#118314 to address the
original issue. Once that PR is merged, we can lift this restriction
as well.
Copy link
Member

@ergawy ergawy left a comment

Choose a reason for hiding this comment

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

LGTM!

@abidh abidh merged commit bfd3e25 into llvm:main Feb 13, 2025
8 checks passed
flovent pushed a commit to flovent/llvm-project that referenced this pull request Feb 13, 2025
We currently drop the `DeclareOp` which are not in the entry block of
the function. This was done to side step a problem with the variables in
OpenMP target region. Now that issue has been addressed in llvm#118314 so we
can lift this restriction as well.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
We currently drop the `DeclareOp` which are not in the entry block of
the function. This was done to side step a problem with the variables in
OpenMP target region. Now that issue has been addressed in llvm#118314 so we
can lift this restriction as well.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
We currently drop the `DeclareOp` which are not in the entry block of
the function. This was done to side step a problem with the variables in
OpenMP target region. Now that issue has been addressed in llvm#118314 so we
can lift this restriction as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants