[flang] Add debug information for module variables.

[abidh]

This PR add debug info for module variables. The module variables are added as global variables but their scope is set to module instead of compile unit. The scope of function declared inside a module is also set accordingly.

After this patch, a module variable could be evaluated in the GDB as p helper::gli where helper is name of the module and gli is the name of the variable. A future patch will add the import module functionality which will remove the need to prefix the name with helper::.

The line number where is module is declared is a best guess at the moment as this information is not part of the GlobalOp.

[llvmbot]

@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-mlir

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

Author: Abid Qadeer (abidh)

Changes

This PR add debug info for module variables. The module variables are added as global variables but their scope is set to module instead of compile unit. The scope of function declared inside a module is also set accordingly. After this patch, a module variable could be evaluated in the GDB as `p helper::gli` where helper is name of the module and gli is the name of the variable. A future patch will add the import module functionality which will remove the need to prefix the name with helper::. The line number where is module is declared is a best guess at the moment as this information is not part of the GlobalOp. --- Full diff: https://github.com/llvm/llvm-project/pull/91582.diff 5 Files Affected: - (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+16-1) - (modified) flang/lib/Optimizer/Transforms/AddDebugInfo.cpp (+91-5) - (added) flang/test/Transforms/debug-module-1.f90 (+39) - (added) flang/test/Transforms/debug-module-2.f90 (+37) - (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+7-2) ``````````diff diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp index b4705aa479925..4618643f22479 100644 --- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp +++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp @@ -2694,6 +2694,18 @@ struct GlobalOpConversion : public fir::FIROpConversion { mlir::LogicalResult matchAndRewrite(fir::GlobalOp global, OpAdaptor adaptor, mlir::ConversionPatternRewriter &rewriter) const override { + + mlir::LLVM::DIGlobalVariableExpressionAttr dbgExpr; + + if (auto fusedLoc = mlir::dyn_cast(global.getLoc())) { + if (auto gvAttr = + mlir::dyn_cast_or_null( + fusedLoc.getMetadata())) { + dbgExpr = mlir::LLVM::DIGlobalVariableExpressionAttr::get( + global.getContext(), gvAttr, mlir::LLVM::DIExpressionAttr()); + } + } + auto tyAttr = convertType(global.getType()); if (auto boxType = mlir::dyn_cast(global.getType())) tyAttr = this->lowerTy().convertBoxTypeAsStruct(boxType); @@ -2702,8 +2714,11 @@ struct GlobalOpConversion : public fir::FIROpConversion { assert(attributeTypeIsCompatible(global.getContext(), initAttr, tyAttr)); auto linkage = convertLinkage(global.getLinkName()); auto isConst = global.getConstant().has_value(); + mlir::SymbolRefAttr comdat; + llvm::ArrayRef attrs; auto g = rewriter.create( - loc, tyAttr, isConst, linkage, global.getSymName(), initAttr); + loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0, 0, + false, false, comdat, attrs, dbgExpr); auto module = global->getParentOfType(); // Add comdat if necessary diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp index 908c8fc96f633..0a1acb8b39372 100644 --- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp +++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp @@ -34,6 +34,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" +#include

[kiranchandramohan]

There is a plan for a pass (https://github.com/llvm/llvm-project/pull/73829) that would convert some constants to globals. Would it be OK to mark this as a Global Variable?

[abidh]

The patch adds debug info for other kind of globals (besides module variables) like for eg. x in the following case. Is that the intention? If so a test would be good.

subroutine abc
  integer :: x = 2
  print *, x
end subroutine

This was not intentional. The variable with save attribute should be added as global but with scope limited to the subroutine. I will add a check to skip them in this patch.

On a related note, while processing DeclareOp of such variables, are you aware of anything that will tell me that these variables have save attribute? I ask because I noticed that they are being added as local variable too.

Commonblocks also are globals but I did not see debug for them.

Yes, common blocks are not supported yet.

I am guessing the following points will come in future patches. -> Renamed module variables using use statement. -> Private module variables. Can they be distinguished in DWARF? -> Submodules

Private module variable should work with this patch. I was not aware of the renaming by the use statement syntax. I will add it in my todo list. DWARF has concept of DW_TAG_imported_declaration for such cases. I will have to see if we have enough information to generate it.

[abidh]

There is a plan for a pass (#73829) that would convert some constants to globals. Would it be OK to mark this as a Global Variable?

Currently, we add variable only if deconstructed name kind is fir::NameUniquer::NameKind::VARIABLE. I think that should filter out the generated globals.

[kiranchandramohan]

On a related note, while processing DeclareOp of such variables, are you aware of anything that will tell me that these variables have save attribute? I ask because I noticed that they are being added as local variable too.

I am not aware.

[github-actions[bot]]

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.