[flang] Initial debug info support for local variables. #90905
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, cg-rewrite removes the DeclareOp. As AddDebugInfo runs after that, it cannot process the DeclareOp. My initial plan was to make the AddDebugInfo pass run before the cg-rewrite but that has few issues. 1. Initially I was thinking to use the memref op to carry the variable attr. But as @tblah suggested in the llvm#86939, it makes more sense to carry that information on DeclareOp. It also makes it easy to handle it in codegen and there is no special handling needed for arguments. For that, we need to preserve the DeclareOp till the codegen. 2. Running earlier, we will miss the changes in passes that run between cg-rewrite and codegen. But not removing the DeclareOp in cg-rewrite has the issue that ShapeOp remains and it causes errors during codegen. To solve this problem, I convert DeclareOp to XDeclareOp in cg-rewrite instead of removing it. This was mentioned as possible solution by @jeanPerier in https://reviews.llvm.org/D136254 The conversion follows similar logic as used for other operators in that file. The FortranAttr and CudaAttr are currently not converted but left as TODO when the need arise. A later commit will use the XDeclareOp to extract the variable information.
This commit extracts information about local variables from XDeclareOp and creates DILocalVariableAttr. These are attached to DeclareOp using FusedLoc approach. Codegen can use them to create DbgDeclareOp. I have added tests that checks the debug information in mlir from and also in llvm ir.
The previous placeholder type was basic type with DW_ATE_address encoding. When variables are added, it started causing assertions in the llvm debug info generation logic for some types. It has been changed to an integer type.
abidh
requested review from
tblah,
kiranchandramohan,
jeanPerier,
dpalermo,
kiranktp and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen Author: Abid Qadeer (abidh) ChangesWe need the information in the
But not removing the DeclareOp in cg-rewrite has the issue that ShapeOp The conversion follows similar logic as used for other operators in that Now Currently we only handle very limited types. Rest are given a place holder Patch is 24.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90905.diff 11 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/CGOps.h b/flang/include/flang/Optimizer/CodeGen/CGOps.h
similarity index 94%
rename from flang/lib/Optimizer/CodeGen/CGOps.h
rename to flang/include/flang/Optimizer/CodeGen/CGOps.h
index b5a6d5bb9a9e6f..df909d9ee81cb4 100644
--- a/flang/lib/Optimizer/CodeGen/CGOps.h
+++ b/flang/include/flang/Optimizer/CodeGen/CGOps.h
@@ -13,6 +13,7 @@
#ifndef OPTIMIZER_CODEGEN_CGOPS_H
#define OPTIMIZER_CODEGEN_CGOPS_H
+#include "flang/Optimizer/Dialect/FIRAttr.h"
#include "flang/Optimizer/Dialect/FIRType.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
diff --git a/flang/include/flang/Optimizer/CodeGen/CGOps.td b/flang/include/flang/Optimizer/CodeGen/CGOps.td
index 35e70fa2ffa3fb..c375edee1fa77f 100644
--- a/flang/include/flang/Optimizer/CodeGen/CGOps.td
+++ b/flang/include/flang/Optimizer/CodeGen/CGOps.td
@@ -16,6 +16,8 @@
include "mlir/IR/SymbolInterfaces.td"
include "flang/Optimizer/Dialect/FIRTypes.td"
+include "flang/Optimizer/Dialect/FIRAttr.td"
+include "mlir/IR/BuiltinAttributes.td"
def fircg_Dialect : Dialect {
let name = "fircg";
@@ -202,4 +204,36 @@ def fircg_XArrayCoorOp : fircg_Op<"ext_array_coor", [AttrSizedOperandSegments]>
}];
}
+// Extended Declare operation.
+def fircg_XDeclareOp : fircg_Op<"ext_declare", [AttrSizedOperandSegments]> {
+ let summary = "for internal conversion only";
+
+ let description = [{
+ Prior to lowering to LLVM IR dialect, a DeclareOp will
+ be converted to an extended DeclareOp.
+ }];
+
+ let arguments = (ins
+ AnyRefOrBox:$memref,
+ Variadic<AnyIntegerType>:$shape,
+ Variadic<AnyIntegerType>:$shift,
+ Variadic<AnyIntegerType>:$typeparams,
+ Optional<fir_DummyScopeType>:$dummy_scope,
+ Builtin_StringAttr:$uniq_name
+ );
+ let results = (outs AnyRefOrBox);
+
+ let assemblyFormat = [{
+ $memref (`(` $shape^ `)`)? (`origin` $shift^)? (`typeparams` $typeparams^)?
+ (`dummy_scope` $dummy_scope^)?
+ attr-dict `:` functional-type(operands, results)
+ }];
+
+ let extraClassDeclaration = [{
+ // Shape is optional, but if it exists, it will be at offset 1.
+ unsigned shapeOffset() { return 1; }
+ unsigned shiftOffset() { return shapeOffset() + getShape().size(); }
+ }];
+}
+
#endif
diff --git a/flang/lib/Optimizer/CodeGen/CGOps.cpp b/flang/lib/Optimizer/CodeGen/CGOps.cpp
index 44d07d26dd2b68..6b8ba74525556e 100644
--- a/flang/lib/Optimizer/CodeGen/CGOps.cpp
+++ b/flang/lib/Optimizer/CodeGen/CGOps.cpp
@@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//
-#include "CGOps.h"
+#include "flang/Optimizer/CodeGen/CGOps.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/Dialect/FIRType.h"
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index b4705aa4799258..163b1a0832273c 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -12,7 +12,7 @@
#include "flang/Optimizer/CodeGen/CodeGen.h"
-#include "CGOps.h"
+#include "flang/Optimizer/CodeGen/CGOps.h"
#include "flang/Optimizer/CodeGen/CodeGenOpenMP.h"
#include "flang/Optimizer/CodeGen/FIROpPatterns.h"
#include "flang/Optimizer/CodeGen/TypeConverter.h"
@@ -170,6 +170,28 @@ genAllocationScaleSize(OP op, mlir::Type ity,
return nullptr;
}
+namespace {
+struct DeclareOpConversion : public fir::FIROpConversion<fir::cg::XDeclareOp> {
+public:
+ using FIROpConversion::FIROpConversion;
+ mlir::LogicalResult
+ matchAndRewrite(fir::cg::XDeclareOp declareOp, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const override {
+ auto memRef = adaptor.getOperands()[0];
+ if (auto fusedLoc = mlir::dyn_cast<mlir::FusedLoc>(declareOp.getLoc())) {
+ if (auto varAttr =
+ mlir::dyn_cast_or_null<mlir::LLVM::DILocalVariableAttr>(
+ fusedLoc.getMetadata())) {
+ rewriter.create<mlir::LLVM::DbgDeclareOp>(memRef.getLoc(), memRef,
+ varAttr, nullptr);
+ }
+ }
+ rewriter.replaceOp(declareOp, memRef);
+ return mlir::success();
+ }
+};
+} // namespace
+
namespace {
/// convert to LLVM IR dialect `alloca`
struct AllocaOpConversion : public fir::FIROpConversion<fir::AllocaOp> {
@@ -3714,19 +3736,19 @@ void fir::populateFIRToLLVMConversionPatterns(
BoxOffsetOpConversion, BoxProcHostOpConversion, BoxRankOpConversion,
BoxTypeCodeOpConversion, BoxTypeDescOpConversion, CallOpConversion,
CmpcOpConversion, ConstcOpConversion, ConvertOpConversion,
- CoordinateOpConversion, DTEntryOpConversion, DivcOpConversion,
- EmboxOpConversion, EmboxCharOpConversion, EmboxProcOpConversion,
- ExtractValueOpConversion, FieldIndexOpConversion, FirEndOpConversion,
- FreeMemOpConversion, GlobalLenOpConversion, GlobalOpConversion,
- HasValueOpConversion, InsertOnRangeOpConversion, InsertValueOpConversion,
- IsPresentOpConversion, LenParamIndexOpConversion, LoadOpConversion,
- MulcOpConversion, NegcOpConversion, NoReassocOpConversion,
- SelectCaseOpConversion, SelectOpConversion, SelectRankOpConversion,
- SelectTypeOpConversion, ShapeOpConversion, ShapeShiftOpConversion,
- ShiftOpConversion, SliceOpConversion, StoreOpConversion,
- StringLitOpConversion, SubcOpConversion, TypeDescOpConversion,
- TypeInfoOpConversion, UnboxCharOpConversion, UnboxProcOpConversion,
- UndefOpConversion, UnreachableOpConversion,
+ CoordinateOpConversion, DTEntryOpConversion, DeclareOpConversion,
+ DivcOpConversion, EmboxOpConversion, EmboxCharOpConversion,
+ EmboxProcOpConversion, ExtractValueOpConversion, FieldIndexOpConversion,
+ FirEndOpConversion, FreeMemOpConversion, GlobalLenOpConversion,
+ GlobalOpConversion, HasValueOpConversion, InsertOnRangeOpConversion,
+ InsertValueOpConversion, IsPresentOpConversion, LenParamIndexOpConversion,
+ LoadOpConversion, MulcOpConversion, NegcOpConversion,
+ NoReassocOpConversion, SelectCaseOpConversion, SelectOpConversion,
+ SelectRankOpConversion, SelectTypeOpConversion, ShapeOpConversion,
+ ShapeShiftOpConversion, ShiftOpConversion, SliceOpConversion,
+ StoreOpConversion, StringLitOpConversion, SubcOpConversion,
+ TypeDescOpConversion, TypeInfoOpConversion, UnboxCharOpConversion,
+ UnboxProcOpConversion, UndefOpConversion, UnreachableOpConversion,
UnrealizedConversionCastOpConversion, XArrayCoorOpConversion,
XEmboxOpConversion, XReboxOpConversion, ZeroOpConversion>(converter,
options);
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 5bd3ec8d18450e..b281f45180bb9a 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -12,8 +12,8 @@
#include "flang/Optimizer/CodeGen/CodeGen.h"
-#include "CGOps.h"
#include "flang/Optimizer/Builder/Todo.h" // remove when TODO's are done
+#include "flang/Optimizer/CodeGen/CGOps.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/Dialect/FIRType.h"
@@ -276,7 +276,29 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
mlir::LogicalResult
matchAndRewrite(fir::DeclareOp declareOp,
mlir::PatternRewriter &rewriter) const override {
- rewriter.replaceOp(declareOp, declareOp.getMemref());
+ auto loc = declareOp.getLoc();
+ llvm::SmallVector<mlir::Value> shapeOpers;
+ llvm::SmallVector<mlir::Value> shiftOpers;
+ if (auto shapeVal = declareOp.getShape()) {
+ if (auto shapeOp = mlir::dyn_cast<fir::ShapeOp>(shapeVal.getDefiningOp()))
+ populateShape(shapeOpers, shapeOp);
+ else if (auto shiftOp =
+ mlir::dyn_cast<fir::ShapeShiftOp>(shapeVal.getDefiningOp()))
+ populateShapeAndShift(shapeOpers, shiftOpers, shiftOp);
+ else if (auto shiftOp =
+ mlir::dyn_cast<fir::ShiftOp>(shapeVal.getDefiningOp()))
+ populateShift(shiftOpers, shiftOp);
+ else
+ return mlir::failure();
+ }
+ // FIXME: Add FortranAttrs and CudaAttrs
+ auto xDeclOp = rewriter.create<fir::cg::XDeclareOp>(
+ loc, declareOp.getType(), declareOp.getMemref(), shapeOpers, shiftOpers,
+ declareOp.getTypeparams(), declareOp.getDummyScope(),
+ declareOp.getUniqName());
+ LLVM_DEBUG(llvm::dbgs()
+ << "rewriting " << declareOp << " to " << xDeclOp << '\n');
+ rewriter.replaceOp(declareOp, xDeclOp.getOperation()->getResults());
return mlir::success();
}
};
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 908c8fc96f633e..253477ef175e1f 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -15,6 +15,7 @@
#include "flang/Common/Version.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/CodeGen/CGOps.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/Dialect/FIRType.h"
@@ -45,13 +46,44 @@ namespace fir {
namespace {
class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
+ void handleDeclareOp(fir::cg::XDeclareOp declOp,
+ mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scopeAttr,
+ fir::DebugTypeGenerator &typeGen, uint32_t &argNo);
+
public:
AddDebugInfoPass(fir::AddDebugInfoOptions options) : Base(options) {}
void runOnOperation() override;
};
+static uint32_t getLineFromLoc(mlir::Location loc) {
+ uint32_t line = 1;
+ if (auto fileLoc = mlir::dyn_cast<mlir::FileLineColLoc>(loc))
+ line = fileLoc.getLine();
+ return line;
+}
+
} // namespace
+void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
+ mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scopeAttr,
+ fir::DebugTypeGenerator &typeGen,
+ uint32_t &argNo) {
+ mlir::MLIRContext *context = &getContext();
+ mlir::OpBuilder builder(context);
+
+ bool isLocal = (declOp.getMemref().getDefiningOp() != nullptr);
+ auto tyAttr = typeGen.convertType(fir::unwrapRefType(declOp.getType()),
+ fileAttr, scopeAttr, declOp.getLoc());
+ auto result = fir::NameUniquer::deconstruct(declOp.getUniqName());
+ auto localVarAttr = mlir::LLVM::DILocalVariableAttr::get(
+ context, scopeAttr, mlir::StringAttr::get(context, result.second.name),
+ fileAttr, getLineFromLoc(declOp.getLoc()), isLocal ? 0 : argNo++,
+ /* alignInBits*/ 0, tyAttr);
+ declOp->setLoc(builder.getFusedLoc({declOp->getLoc()}, localVarAttr));
+}
+
void AddDebugInfoPass::runOnOperation() {
mlir::ModuleOp module = getOperation();
mlir::MLIRContext *context = &getContext();
@@ -144,14 +176,16 @@ void AddDebugInfoPass::runOnOperation() {
subprogramFlags =
subprogramFlags | mlir::LLVM::DISubprogramFlags::Definition;
}
- unsigned line = 1;
- if (auto funcLoc = mlir::dyn_cast<mlir::FileLineColLoc>(l))
- line = funcLoc.getLine();
-
+ unsigned line = getLineFromLoc(l);
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
context, id, compilationUnit, fileAttr, funcName, fullName,
funcFileAttr, line, line, subprogramFlags, subTypeAttr);
funcOp->setLoc(builder.getFusedLoc({funcOp->getLoc()}, spAttr));
+
+ uint32_t argNo = 1;
+ funcOp.walk([&](fir::cg::XDeclareOp declOp) {
+ handleDeclareOp(declOp, fileAttr, spAttr, typeGen, argNo);
+ });
});
}
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index e5b4050dfb2426..64c6547e06e0f9 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -24,11 +24,6 @@ DebugTypeGenerator::DebugTypeGenerator(mlir::ModuleOp m)
LLVM_DEBUG(llvm::dbgs() << "DITypeAttr generator\n");
}
-static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) {
- return mlir::LLVM::DIBasicTypeAttr::get(
- context, llvm::dwarf::DW_TAG_base_type, "void", 32, 1);
-}
-
static mlir::LLVM::DITypeAttr genBasicType(mlir::MLIRContext *context,
mlir::StringAttr name,
unsigned bitSize,
@@ -37,6 +32,11 @@ static mlir::LLVM::DITypeAttr genBasicType(mlir::MLIRContext *context,
context, llvm::dwarf::DW_TAG_base_type, name, bitSize, decoding);
}
+static mlir::LLVM::DITypeAttr genPlaceholderType(mlir::MLIRContext *context) {
+ return genBasicType(context, mlir::StringAttr::get(context, "integer"), 32,
+ llvm::dwarf::DW_ATE_signed);
+}
+
mlir::LLVM::DITypeAttr
DebugTypeGenerator::convertType(mlir::Type Ty, mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scope,
diff --git a/flang/test/Fir/declare-codegen.fir b/flang/test/Fir/declare-codegen.fir
index 9d68d3b2f9d4de..841dc06ed66627 100644
--- a/flang/test/Fir/declare-codegen.fir
+++ b/flang/test/Fir/declare-codegen.fir
@@ -17,7 +17,8 @@ func.func private @bar(%arg0: !fir.ref<!fir.array<12x23xi32>>)
// CHECK-LABEL: func.func @test(
// CHECK-SAME: %[[arg0:.*]]: !fir.ref<!fir.array<12x23xi32>>) {
-// CHECK-NEXT: fir.call @bar(%[[arg0]]) : (!fir.ref<!fir.array<12x23xi32>>) -> ()
+// CHECK: fircg.ext_declare
+
func.func @useless_shape_with_duplicate_extent_operand(%arg0: !fir.ref<!fir.array<3x3xf32>>) {
%c3 = arith.constant 3 : index
@@ -27,4 +28,4 @@ func.func @useless_shape_with_duplicate_extent_operand(%arg0: !fir.ref<!fir.arra
}
// CHECK-LABEL: func.func @useless_shape_with_duplicate_extent_operand(
-// CHECK-NEXT: return
+// CHECK: fircg.ext_declare
diff --git a/flang/test/Fir/dummy-scope-codegen.fir b/flang/test/Fir/dummy-scope-codegen.fir
index caef3c1b257832..2b80d1bb62ea44 100644
--- a/flang/test/Fir/dummy-scope-codegen.fir
+++ b/flang/test/Fir/dummy-scope-codegen.fir
@@ -6,4 +6,4 @@ func.func @dummy_scope(%arg0: !fir.ref<f32>) {
return
}
// CHECK-LABEL: func.func @dummy_scope(
-// CHECK-NEXT: return
+// CHECK: fircg.ext_declare
diff --git a/flang/test/Transforms/debug-local-var-2.f90 b/flang/test/Transforms/debug-local-var-2.f90
new file mode 100644
index 00000000000000..15b9b148492e14
--- /dev/null
+++ b/flang/test/Transforms/debug-local-var-2.f90
@@ -0,0 +1,91 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
+
+! This tests checks the debug information for local variables in llvm IR.
+
+! CHECK-LABEL: define void @_QQmain
+! CHECK-DAG: %[[AL11:.*]] = alloca i32
+! CHECK-DAG: %[[AL12:.*]] = alloca i64
+! CHECK-DAG: %[[AL13:.*]] = alloca i8
+! CHECK-DAG: %[[AL14:.*]] = alloca i32
+! CHECK-DAG: %[[AL15:.*]] = alloca float
+! CHECK-DAG: %[[AL16:.*]] = alloca double
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL11]], metadata ![[I4:.*]], metadata !DIExpression())
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL12]], metadata ![[I8:.*]], metadata !DIExpression())
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL13]], metadata ![[L1:.*]], metadata !DIExpression())
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL14]], metadata ![[L4:.*]], metadata !DIExpression())
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL15]], metadata ![[R4:.*]], metadata !DIExpression())
+! CHECK-DAG: call void @llvm.dbg.declare(metadata ptr %[[AL16]], metadata ![[R8:.*]], metadata !DIExpression())
+! CHECK-LABEL: }
+
+! CHECK-LABEL: define {{.*}}i64 @_QFPfn1
+! CHECK-SAME: (ptr %[[ARG1:.*]], ptr %[[ARG2:.*]], ptr %[[ARG3:.*]])
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[ARG1]], metadata ![[A1:.*]], metadata !DIExpression())
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[ARG2]], metadata ![[B1:.*]], metadata !DIExpression())
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[ARG3]], metadata ![[C1:.*]], metadata !DIExpression())
+! CHECK-DAG: %[[AL2:.*]] = alloca i64
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[AL2]], metadata ![[RES1:.*]], metadata !DIExpression())
+! CHECK-LABEL: }
+
+! CHECK-LABEL: define {{.*}}i32 @_QFPfn2
+! CHECK-SAME: (ptr %[[FN2ARG1:.*]], ptr %[[FN2ARG2:.*]], ptr %[[FN2ARG3:.*]])
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[FN2ARG1]], metadata ![[A2:.*]], metadata !DIExpression())
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[FN2ARG2]], metadata ![[B2:.*]], metadata !DIExpression())
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[FN2ARG3]], metadata ![[C2:.*]], metadata !DIExpression())
+! CHECK-DAG: %[[AL3:.*]] = alloca i32
+! CHECK-DAG: tail call void @llvm.dbg.declare(metadata ptr %[[AL3]], metadata ![[RES2:.*]], metadata !DIExpression())
+! CHECK-LABEL: }
+
+program mn
+! CHECK-DAG: ![[MAIN:.*]] = distinct !DISubprogram(name: "_QQmain", {{.*}})
+
+! CHECK-DAG: ![[TYI32:.*]] = !DIBasicType(name: "integer", size: 32, encoding: DW_ATE_signed)
+! CHECK-DAG: ![[TYI64:.*]] = !DIBasicType(name: "integer", size: 64, encoding: DW_ATE_signed)
+! CHECK-DAG: ![[TYL8:.*]] = !DIBasicType(name: "logical", size: 8, encoding: DW_ATE_boolean)
+! CHECK-DAG: ![[TYL32:.*]] = !DIBasicType(name: "logical", size: 32, encoding: DW_ATE_boolean)
+! CHECK-DAG: ![[TYR32:.*]] = !DIBasicType(name: "real", size: 32, encoding: DW_ATE_float)
+! CHECK-DAG: ![[TYR64:.*]] = !DIBasicType(name: "real", size: 64, encoding: DW_ATE_float)
+
+! CHECK-DAG: ![[I4]] = !DILocalVariable(name: "i4", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYI32]])
+! CHECK-DAG: ![[I8]] = !DILocalVariable(name: "i8", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYI64]])
+! CHECK-DAG: ![[R4]] = !DILocalVariable(name: "r4", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYR32]])
+! CHECK-DAG: ![[R8]] = !DILocalVariable(name: "r8", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYR64]])
+! CHECK-DAG: ![[L1]] = !DILocalVariable(name: "l1", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYL8]])
+! CHECK-DAG: ![[L4]] = !DILocalVariable(name: "l4", scope: ![[MAIN]], file: !{{.*}}, line: [[@LINE+6]], type: ![[TYL32]])
+ integer(kind=4) :: i4
+ integer(kind=8) :: i8
+ real(kind=4) :: r4
+ real(kind=8) :: r8
+ logical(kind=1) :: l1
+ logical(kind=4) :: l4
+
+ i8 = fn1(i4, r8, l1)
+ i4 = fn2(i8, r4, l4)
+contains
+! CHECK-DAG: ![[FN1:.*]] = distinct !DISubprogram(name: "fn1", {{.*}})
+! CHECK-DAG: ![[A1]] = !DILocalVariable(name: "a1", arg: 1, scope: ![[FN1]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYI32]])
+! CHECK-DAG: ![[B1]] = !DILocalVariable(name: "b1", arg: 2, scope: ![[FN1]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYR64]])
+! CHECK-DAG: ![[C1]] = !DILocalVariable(name: "c1", arg: 3, scope: ![[FN1]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYL8]])
+! CHECK-DAG: ![[RES1]] = !DILocalVariable(name: "res1", scope: ![[FN1]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYI64]])
+ function fn1(a1, b1, c1) result (res1)
+ integer(kind=4), intent(in) :: a1
+ real(kind=8), intent(in) :: b1
+ logical(kind=1), intent(in) :: c1
+ integer(kind=8) :: res1
+
+ res1 = a1 + b1
+ end function
+
+! CHECK-DAG: ![[FN2:.*]] = distinct !DISubprogram(name: "fn2", {{.*}})
+! CHECK-DAG: ![[A2]] = !DILocalVariable(name: "a2", arg: 1, scope: ![[FN2]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYI64]])
+! CHECK-DAG: ![[B2]] = !DILocalVariable(name: "b2", arg: 2, scope: ![[FN2]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYR32]])
+! CHECK-DAG: ![[C2]] = !DILocalVariable(name: "c2", arg: 3, scope: ![[FN2]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYL32]])
+! CHECK-DAG: ![[RES2]] = !DILocalVariable(name: "res2", scope: ![[FN2]], file: !{{.*}}, line: [[@LINE+5]], type: ![[TYI32]])
+ function fn2(a2, b2, c2) result (res2)
+ integer(kind=8), intent(in) :: a2
+ real(kind=4), ...
[truncated]
|
tblah
reviewed
May 3, 2024
| mlir::MLIRContext *context = &getContext(); | ||
| mlir::OpBuilder builder(context); | ||
|
|
||
| bool isLocal = (declOp.getMemref().getDefiningOp() != nullptr); |
There was a problem hiding this comment.
I think block arguments would have no defining op. This could happen after structured control flow has been lowered to blocks and branches like in LLVM IR.
Off the top of my head, I think all of the source code variable declarations would end up in the entry block to the function and so wouldn't have this problem. It might be possible that declare operations for compiler temporaries come via block arguments (e.g. after control flow). I think these might then be mislabeled as dummy arguments, and end up incrementing argNo.
jeanPerier
reviewed
May 3, 2024
| } | ||
| // FIXME: Add FortranAttrs and CudaAttrs | ||
| auto xDeclOp = rewriter.create<fir::cg::XDeclareOp>( | ||
| loc, declareOp.getType(), declareOp.getMemref(), shapeOpers, shiftOpers, |
There was a problem hiding this comment.
Maybe this should only be created if there is a DILocalVariableAttr fused with the location of the DeclareOp.
Creating MLIR operations, especially with many arguments, has a compile time cost (both memory and time), so I would be inclined to start by only doing it when we know it is needed, which is easy here currently. That way, there is no extra compile time cost for filed compiled without -g.
Future use cases can enable this translation when they arise.
There was a problem hiding this comment.
This pre-cg-rewrite runs before AddDebugInfo so we would not have created FusedLoc by that time. I am thinking that we could run a cleanup pass before pre-cg-rewrite that removes the DeclareOp if debug info is disabled. Does that sound like the right approach to you?
There was a problem hiding this comment.
Good point. I think adding a pass option "preserveDeclare" to pre-cg-rewrite passwould be simpler and cheaper then. It can be set in createDefaultFIRCodeGenPassPipeline here given the debug level.
There was a problem hiding this comment.
Thanks for the suggestions. I have added the option in 2c566dd. Also updated the tests to check with this option turned off and on.
| mlir::MLIRContext *context = &getContext(); | ||
| mlir::OpBuilder builder(context); | ||
|
|
||
| bool isLocal = (declOp.getMemref().getDefiningOp() != nullptr); |
There was a problem hiding this comment.
I find isLocal a bit confusing here because global variables would also have "isLocal" set to true (since they are the result of a fir.address_of).
I guess reversing the logic and using "isDummyArgument" would described more what this is about. Looking for the existence of a defining op is also not solid because there are cases (e.g.) where there will be a little bit of argument "cooking" before the hlfir.declare (converts/unboxing). For instance in:
subroutine foo(c)
character(10) :: c
end subroutine
Also, I am not sure what "argNo" ends-up being used for in the DWARF, but it is likely not accurate since hlfir.declare are created alphabetically (except when a variable "B" appears in the specification expression of another variable "A", in which case "B" declare is created before "A").
Does the position must be the Fortran argument position, or the one in the LLVM signature? These may differ since some Fortran function results may be passed by pointer via the first argument in the LLVM signature, and some Fortran arguments are split (The length of character arguments is passed after all Fortran arguments).
So we may need to add some info on the fir/hlfir declare to indicate something is a dummy argument, and what is its position.
Do you also know what should happen after inlining?
There was a problem hiding this comment.
FYI, I am working on adding dummy_scope operand for [hl]fir.declare. It will only be set for the declare operations of the dummy arguments (as they are in the source code). Though I am not sure if that would be helpful here.
There was a problem hiding this comment.
The argNo value we pass will determine the order in which debugger will show the variables.
function fn1(c1, a1, b1) result (res1)
...
end function
(GDB) frame
#0 fn1 (c1=.FALSE., a1=1, b1=2.1000000000000001) at ../test1.f90:20
You were right that DeclareOP were in alphabetical order. I have now used BlockArgument::getArgNumber and it seems to return the correct order value. I am also using cast to BlockArgument to check if it is a dummy argument. But the problem of argument cooking in certain cases remain. I added a FIXME for that. Once I know all the cases where it can happen, it probably should not be too difficult to handle.
From inlining, you mean inlining that can happen before we process DeclareOp or after. If it happens after then I think it is up to that pass to adjust any metadata accordingly. Are there instances where inlining will happen before AddDebugInfo runs?
There was a problem hiding this comment.
MLIR inlining is not quite supported in FIR dialect, but you can experiment with some cases (some of them may not work) using flang-new -O3 -mmlir -inline-all=true, e.g. for this test:
subroutine test(x, y)
real, target :: x, y
x = y ! may alias
call inner(x, y)
contains
subroutine inner(x, y)
real :: x, y
x = y ! may not alias
end subroutine inner
end subroutine test
I get this MLIR:
// -----// IR Dump After Inliner (inline) //----- //
module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", fir.target_cpu = "x86-64", llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128", llvm.target_triple = "x86_64-unknown-linux-gnu"} {
func.func @_QPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "x", fir.target}, %arg1: !fir.ref<f32> {fir.bindc_name = "y", fir.target}) {
%0 = fir.dummy_scope : !fir.dscope
%1 = fir.declare %arg0 dummy_scope %0 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEx"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
%2 = fir.declare %arg1 dummy_scope %0 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEy"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
%3 = fir.load %2 : !fir.ref<f32>
fir.store %3 to %1 : !fir.ref<f32>
%4 = fir.dummy_scope : !fir.dscope
%5 = fir.declare %1 dummy_scope %4 {uniq_name = "_QFtestFinnerEx"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
%6 = fir.declare %2 dummy_scope %4 {uniq_name = "_QFtestFinnerEy"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
%7 = fir.load %6 : !fir.ref<f32>
fir.store %7 to %5 : !fir.ref<f32>
return
}
}
There was a problem hiding this comment.
Thanks. I looked at the IR in such case and it seems that function that gets inlined loses any FuncOp. Name in DeclareOp and CallSiteLoc may provide some clues but work will be required to retain enough information during inlining to enable AddDebugInfo to do its job. In any case, our first target should be to get the debug working with -O0.
Keeping fir.declare would require handling fir.shape type and operations in codegen because fir.shape/shape_shift are currently "virtual" type with no implementations. |
Following changes were done. 1. Discard variables if they are not of NameKind::VARIABLE kind. 2. Use casting to BlockArgument to check if it is a dummy argument and use getArgNumber() to get its number.
When this option is true, DeclareOp is converted to XDeclareOp and this OP is used later for debug info generation. When it is false, DeclareOp is removed.
|
Any further comments on this PR? |
The DeclareOp is also generated for module variable used in a function. We reject them to avoid creating local variables for them.
tblah
approved these changes
May 13, 2024
|
I am seeing an assertion that I believe is related to this commit. File 90905.f90 |
Thanks for reporting this. I am investigating. |
psteinfeld
added a commit
to psteinfeld/llvm-project
that referenced
this pull request
May 15, 2024
…#90905)" This reverts commit 61da636. See comments in llvm#90905.
psteinfeld
added a commit
that referenced
this pull request
May 15, 2024
|
The problem was that variables were being generated even when user just asked for line-tables. I have fixed it now. I will re-land it after testing. |
Thanks, @abidh. Let me know if you want me to test something. |
I have opened #92304 which should fix the issue that caused assertion. If you could give it a go then that would be great. |
abidh
added a commit
that referenced
this pull request
May 16, 2024
This is same as #90905 with an added fix. The issue was that we generated variable info even when user asked for line-tables-only. This caused llvm dwarf generation code to fail an assertion as it expected an empty variable list. Fixed by not generating debug info for variables when user wants only line table. I also updated a test check for this case.
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Jun 5, 2024
…0b75adbc1 Local branch amd-gfx cac0b75 Merged main:398162ddbcf741c49e86bef2ef4aaa3fd0213916 into amd-gfx:f8acc62d410f Remote branch main 61da636 [flang] Initial debug info support for local variables. (llvm#90905)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We need the information in the
DeclareOpto generate debug informationfor variables. Currently, cg-rewrite removes the
DeclareOp. AsAddDebugInforuns after that, it cannot process theDeclareOp. My initialplan was to make the
AddDebugInfopass run before the cg-rewritebut that has few issues.
Initially I was thinking to use the memref op to carry the variable
attr. But as @tblah suggested in the [flang] Add design document for debug info generation. #86939, it makes more sense to
carry that information on
DeclareOp. It also makes it easy to handle itin codegen and there is no special handling needed for arguments. For
this reason, we need to preserve the
DeclareOptill the codegen.Running earlier, we will miss the changes in passes that run between
cg-rewrite and codegen.
But not removing the DeclareOp in cg-rewrite has the issue that ShapeOp
remains and it causes errors during codegen. To solve this problem, I
convert DeclareOp to XDeclareOp in cg-rewrite instead of removing
it. This was mentioned as possible solution by @jeanPerier in
https://reviews.llvm.org/D136254
The conversion follows similar logic as used for other operators in that
file. The FortranAttr and CudaAttr are currently not converted but left
as TODO when the need arise.
Now
AddDebugInfopass can extracts information about local variablesfrom
XDeclareOpand createsDILocalVariableAttr. These are attachedto
XDeclareOpusingFusedLocapproach. Codegen can use them tocreate
DbgDeclareOp. I have added tests that checks the debuginformation in mlir from and also in llvm ir.
Currently we only handle very limited types. Rest are given a place holder
type. The previous placeholder type was basic type with
DW_ATE_addressencoding. When variables are added, it started causing assertions in
the llvm debug info generation logic for some types. It has been changes to
an interger type to prevent these issues until we handle those types properly.