[flang][debug] Make common blocks data extraction more robust. #168752
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
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen Author: Abid Qadeer (abidh) ChangesOur current implementation for extracting information about common block required traversal of FIR which was not ideal but previously there was no other way to obtain that information. The Full diff: https://github.com/llvm/llvm-project/pull/168752.diff 5 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIRCG/CGOps.td b/flang/include/flang/Optimizer/Dialect/FIRCG/CGOps.td
index 04f839386498c..1ddd2703bc448 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRCG/CGOps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRCG/CGOps.td
@@ -229,12 +229,16 @@ def fircg_XDeclareOp : fircg_Op<"ext_declare", [AttrSizedOperandSegments]> {
let arguments = (ins AnyRefOrBox:$memref, Variadic<AnyIntegerType>:$shape,
Variadic<AnyIntegerType>:$shift, Variadic<AnyIntegerType>:$typeparams,
- Optional<fir_DummyScopeType>:$dummy_scope, Builtin_StringAttr:$uniq_name);
+ Optional<fir_DummyScopeType>:$dummy_scope,
+ Optional<AnyReferenceLike>:$storage,
+ DefaultValuedAttr<UI64Attr, "0">:$storage_offset,
+ Builtin_StringAttr:$uniq_name);
let results = (outs AnyRefOrBox);
let assemblyFormat = [{
$memref (`(` $shape^ `)`)? (`origin` $shift^)? (`typeparams` $typeparams^)?
(`dummy_scope` $dummy_scope^)?
+ (`storage` `(` $storage^ `[` $storage_offset `]` `)`)?
attr-dict `:` functional-type(operands, results)
}];
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 1b1d43c11c707..2672a565c7ff2 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -306,6 +306,7 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
auto xDeclOp = fir::cg::XDeclareOp::create(
rewriter, loc, declareOp.getType(), declareOp.getMemref(), shapeOpers,
shiftOpers, declareOp.getTypeparams(), declareOp.getDummyScope(),
+ declareOp.getStorage(), declareOp.getStorageOffset(),
declareOp.getUniqName());
LLVM_DEBUG(llvm::dbgs()
<< "rewriting " << declareOp << " to " << xDeclOp << '\n');
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index e006d2e878fd8..00fe2ddcdfef2 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -144,62 +144,72 @@ bool AddDebugInfoPass::createCommonBlockGlobal(
fir::DebugTypeGenerator &typeGen, mlir::SymbolTable *symbolTable) {
mlir::MLIRContext *context = &getContext();
mlir::OpBuilder builder(context);
- std::optional<std::int64_t> optint;
- mlir::Operation *op = declOp.getMemref().getDefiningOp();
- if (auto conOp = mlir::dyn_cast_if_present<fir::ConvertOp>(op))
- op = conOp.getValue().getDefiningOp();
+ std::optional<std::int64_t> offset;
+ mlir::Value storage = declOp.getStorage();
+ if (!storage)
+ return false;
+
+ // Extract offset from storage_offset attribute
+ uint64_t storageOffset = declOp.getStorageOffset();
+ if (storageOffset != 0)
+ offset = static_cast<std::int64_t>(storageOffset);
+
+ // Get the GlobalOp from the storage value.
+ // The storage may be wrapped in ConvertOp, so unwrap it first.
+ mlir::Operation *storageOp = storage.getDefiningOp();
+ if (auto convertOp = mlir::dyn_cast_if_present<fir::ConvertOp>(storageOp))
+ storageOp = convertOp.getValue().getDefiningOp();
+
+ auto addrOfOp = mlir::dyn_cast_if_present<fir::AddrOfOp>(storageOp);
+ if (!addrOfOp)
+ return false;
+
+ mlir::SymbolRefAttr sym = addrOfOp.getSymbol();
+ fir::GlobalOp global =
+ symbolTable->lookup<fir::GlobalOp>(sym.getRootReference());
+ if (!global)
+ return false;
+
+ // FIXME: We are trying to extract the name of the common block from the
+ // name of the global. As part of mangling, GetCommonBlockObjectName can
+ // add a trailing _ in the name of that global. The demangle function
+ // does not seem to handle such cases. So the following hack is used to
+ // remove the trailing '_'.
+ llvm::StringRef globalSymbol = sym.getRootReference();
+ llvm::StringRef commonName = globalSymbol;
+ if (commonName != Fortran::common::blankCommonObjectName &&
+ !commonName.empty() && commonName.back() == '_')
+ commonName = commonName.drop_back();
+
+ // Create the debug attributes.
+ unsigned line = getLineFromLoc(global.getLoc());
+ mlir::LLVM::DICommonBlockAttr commonBlock =
+ getOrCreateCommonBlockAttr(commonName, fileAttr, scopeAttr, line);
+
+ mlir::LLVM::DITypeAttr diType = typeGen.convertType(
+ fir::unwrapRefType(declOp.getType()), fileAttr, scopeAttr, declOp);
+
+ line = getLineFromLoc(declOp.getLoc());
+ auto gvAttr = mlir::LLVM::DIGlobalVariableAttr::get(
+ context, commonBlock, mlir::StringAttr::get(context, name),
+ declOp.getUniqName(), fileAttr, line, diType,
+ /*isLocalToUnit*/ false, /*isDefinition*/ true, /* alignInBits*/ 0);
+
+ // Create DIExpression for offset if needed
+ mlir::LLVM::DIExpressionAttr expr;
+ if (offset && *offset != 0) {
+ llvm::SmallVector<mlir::LLVM::DIExpressionElemAttr> ops;
+ ops.push_back(mlir::LLVM::DIExpressionElemAttr::get(
+ context, llvm::dwarf::DW_OP_plus_uconst, *offset));
+ expr = mlir::LLVM::DIExpressionAttr::get(context, ops);
+ }
- if (auto cordOp = mlir::dyn_cast_if_present<fir::CoordinateOp>(op)) {
- auto coors = cordOp.getCoor();
- if (coors.size() != 1)
- return false;
- optint = fir::getIntIfConstant(coors[0]);
- if (!optint)
- return false;
- op = cordOp.getRef().getDefiningOp();
- if (auto conOp2 = mlir::dyn_cast_if_present<fir::ConvertOp>(op))
- op = conOp2.getValue().getDefiningOp();
+ auto dbgExpr = mlir::LLVM::DIGlobalVariableExpressionAttr::get(
+ global.getContext(), gvAttr, expr);
+ globalToGlobalExprsMap[global].push_back(dbgExpr);
- if (auto addrOfOp = mlir::dyn_cast_if_present<fir::AddrOfOp>(op)) {
- mlir::SymbolRefAttr sym = addrOfOp.getSymbol();
- if (auto global =
- symbolTable->lookup<fir::GlobalOp>(sym.getRootReference())) {
-
- unsigned line = getLineFromLoc(global.getLoc());
- llvm::StringRef commonName(sym.getRootReference());
- // FIXME: We are trying to extract the name of the common block from the
- // name of the global. As part of mangling, GetCommonBlockObjectName can
- // add a trailing _ in the name of that global. The demangle function
- // does not seem to handle such cases. So the following hack is used to
- // remove the trailing '_'.
- if (commonName != Fortran::common::blankCommonObjectName &&
- commonName.back() == '_')
- commonName = commonName.drop_back();
- mlir::LLVM::DICommonBlockAttr commonBlock =
- getOrCreateCommonBlockAttr(commonName, fileAttr, scopeAttr, line);
- mlir::LLVM::DITypeAttr diType = typeGen.convertType(
- fir::unwrapRefType(declOp.getType()), fileAttr, scopeAttr, declOp);
- line = getLineFromLoc(declOp.getLoc());
- auto gvAttr = mlir::LLVM::DIGlobalVariableAttr::get(
- context, commonBlock, mlir::StringAttr::get(context, name),
- declOp.getUniqName(), fileAttr, line, diType,
- /*isLocalToUnit*/ false, /*isDefinition*/ true, /* alignInBits*/ 0);
- mlir::LLVM::DIExpressionAttr expr;
- if (*optint != 0) {
- llvm::SmallVector<mlir::LLVM::DIExpressionElemAttr> ops;
- ops.push_back(mlir::LLVM::DIExpressionElemAttr::get(
- context, llvm::dwarf::DW_OP_plus_uconst, *optint));
- expr = mlir::LLVM::DIExpressionAttr::get(context, ops);
- }
- auto dbgExpr = mlir::LLVM::DIGlobalVariableExpressionAttr::get(
- global.getContext(), gvAttr, expr);
- globalToGlobalExprsMap[global].push_back(dbgExpr);
- return true;
- }
- }
- }
- return false;
+ return true;
}
void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
diff --git a/flang/test/Fir/declare-codegen.fir b/flang/test/Fir/declare-codegen.fir
index fe8d84ef2d19f..9413525a5dbd3 100644
--- a/flang/test/Fir/declare-codegen.fir
+++ b/flang/test/Fir/declare-codegen.fir
@@ -52,3 +52,23 @@ func.func @unreachable_code(%arg0: !fir.ref<!fir.char<1,10>>) {
// NODECL-NOT: uniq_name = "live_code"
// DECL-LABEL: func.func @unreachable_code(
// DECL: uniq_name = "live_code"
+
+// Test that storage and storage_offset operands are preserved during conversion
+func.func @test_storage_operands() {
+ %c0 = arith.constant 0 : index
+ %c4 = arith.constant 4 : index
+ %0 = fir.address_of(@common_block) : !fir.ref<!fir.array<8xi8>>
+ %1 = fir.coordinate_of %0, %c0 : (!fir.ref<!fir.array<8xi8>>, index) -> !fir.ref<i8>
+ %2 = fir.convert %1 : (!fir.ref<i8>) -> !fir.ref<f32>
+ %3 = fir.declare %2 storage(%0[0]) {uniq_name = "_QFEx"} : (!fir.ref<f32>, !fir.ref<!fir.array<8xi8>>) -> !fir.ref<f32>
+ %4 = fir.coordinate_of %0, %c4 : (!fir.ref<!fir.array<8xi8>>, index) -> !fir.ref<i8>
+ %5 = fir.convert %4 : (!fir.ref<i8>) -> !fir.ref<i32>
+ %6 = fir.declare %5 storage(%0[4]) {uniq_name = "_QFEy"} : (!fir.ref<i32>, !fir.ref<!fir.array<8xi8>>) -> !fir.ref<i32>
+ return
+}
+fir.global @common_block : !fir.array<8xi8>
+
+// DECL-LABEL: func.func @test_storage_operands()
+// DECL: %[[STORAGE:.*]] = fir.address_of(@common_block) : !fir.ref<!fir.array<8xi8>>
+// DECL: fircg.ext_declare {{.*}} storage(%[[STORAGE]][0]) {uniq_name = "_QFEx"}
+// DECL: fircg.ext_declare {{.*}} storage(%[[STORAGE]][4]) {uniq_name = "_QFEy"}
diff --git a/flang/test/Transforms/debug-common-block.fir b/flang/test/Transforms/debug-common-block.fir
index d68b524225df5..1d2beae0e0ef4 100644
--- a/flang/test/Transforms/debug-common-block.fir
+++ b/flang/test/Transforms/debug-common-block.fir
@@ -16,18 +16,18 @@ module {
%1 = fir.convert %0 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%2 = fir.coordinate_of %1, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%3 = fir.convert %2 : (!fir.ref<i8>) -> !fir.ref<f32>
- %4 = fircg.ext_declare %3 {uniq_name = "_QFf1Ex"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc4)
+ %4 = fircg.ext_declare %3 storage(%0[0]) {uniq_name = "_QFf1Ex"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc4)
%5 = fir.address_of(@a_) : !fir.ref<tuple<i32, !fir.array<8xi8>>>
%6 = fir.convert %5 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%7 = fir.coordinate_of %6, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%8 = fir.convert %7 : (!fir.ref<i8>) -> !fir.ref<f32>
- %9 = fircg.ext_declare %8 {uniq_name = "_QFf1Exa"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc5)
+ %9 = fircg.ext_declare %8 storage(%5[0]) {uniq_name = "_QFf1Exa"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc5)
%10 = fir.coordinate_of %1, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%11 = fir.convert %10 : (!fir.ref<i8>) -> !fir.ref<f32>
- %12 = fircg.ext_declare %11 {uniq_name = "_QFf1Ey"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc6)
+ %12 = fircg.ext_declare %11 storage(%0[4]) {uniq_name = "_QFf1Ey"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc6)
%13 = fir.coordinate_of %6, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%14 = fir.convert %13 : (!fir.ref<i8>) -> !fir.ref<f32>
- %15 = fircg.ext_declare %14 {uniq_name = "_QFf1Eya"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc7)
+ %15 = fircg.ext_declare %14 storage(%5[4]) {uniq_name = "_QFf1Eya"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc7)
return
} loc(#loc3)
func.func @f2() {
@@ -40,24 +40,24 @@ module {
%1 = fir.convert %0 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%2 = fir.coordinate_of %1, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%3 = fir.convert %2 : (!fir.ref<i8>) -> !fir.ref<f32>
- %4 = fircg.ext_declare %3 {uniq_name = "_QFf2Ex"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc9)
+ %4 = fircg.ext_declare %3 storage(%0[0]) {uniq_name = "_QFf2Ex"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc9)
%5 = fir.address_of(@a_) : !fir.ref<tuple<i32, !fir.array<8xi8>>>
%6 = fir.convert %5 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%7 = fir.coordinate_of %6, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%8 = fir.convert %7 : (!fir.ref<i8>) -> !fir.ref<f32>
- %9 = fircg.ext_declare %8 {uniq_name = "_QFf2Exa"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc10)
+ %9 = fircg.ext_declare %8 storage(%5[0]) {uniq_name = "_QFf2Exa"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc10)
%10 = fir.coordinate_of %1, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%11 = fir.convert %10 : (!fir.ref<i8>) -> !fir.ref<f32>
- %12 = fircg.ext_declare %11 {uniq_name = "_QFf2Ey"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc11)
+ %12 = fircg.ext_declare %11 storage(%0[4]) {uniq_name = "_QFf2Ey"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc11)
%13 = fir.coordinate_of %6, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%14 = fir.convert %13 : (!fir.ref<i8>) -> !fir.ref<f32>
- %15 = fircg.ext_declare %14 {uniq_name = "_QFf2Eya"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc12)
+ %15 = fircg.ext_declare %14 storage(%5[4]) {uniq_name = "_QFf2Eya"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc12)
%16 = fir.coordinate_of %1, %c8 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%17 = fir.convert %16 : (!fir.ref<i8>) -> !fir.ref<f32>
- %18 = fircg.ext_declare %17 {uniq_name = "_QFf2Ez"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc13)
+ %18 = fircg.ext_declare %17 storage(%0[8]) {uniq_name = "_QFf2Ez"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc13)
%19 = fir.coordinate_of %6, %c8 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%20 = fir.convert %19 : (!fir.ref<i8>) -> !fir.ref<f32>
- %21 = fircg.ext_declare %20 {uniq_name = "_QFf2Eza"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc14)
+ %21 = fircg.ext_declare %20 storage(%5[8]) {uniq_name = "_QFf2Eza"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc14)
return
} loc(#loc8)
func.func @f3() {
@@ -69,12 +69,12 @@ module {
%1 = fir.convert %0 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%2 = fir.coordinate_of %1, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%3 = fir.convert %2 : (!fir.ref<i8>) -> !fir.ref<i32>
- %4 = fircg.ext_declare %3 {uniq_name = "_QFf3Ex"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc16)
+ %4 = fircg.ext_declare %3 storage(%0[0]) {uniq_name = "_QFf3Ex"} : (!fir.ref<i32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<i32> loc(#loc16)
%5 = fir.address_of(@a_) : !fir.ref<tuple<i32, !fir.array<8xi8>>>
%6 = fir.convert %5 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%7 = fir.coordinate_of %6, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%8 = fir.convert %7 : (!fir.ref<i8>) -> !fir.ref<i32>
- %9 = fircg.ext_declare %8 {uniq_name = "_QFf3Exa"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc17)
+ %9 = fircg.ext_declare %8 storage(%5[0]) {uniq_name = "_QFf3Exa"} : (!fir.ref<i32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<i32> loc(#loc17)
return
} loc(#loc15)
func.func @test() {
@@ -87,24 +87,24 @@ module {
%1 = fir.convert %0 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%2 = fir.coordinate_of %1, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%3 = fir.convert %2 : (!fir.ref<i8>) -> !fir.ref<f32>
- %4 = fircg.ext_declare %3 {uniq_name = "_QFEv1"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc19)
+ %4 = fircg.ext_declare %3 storage(%0[0]) {uniq_name = "_QFEv1"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc19)
%5 = fir.coordinate_of %1, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%6 = fir.convert %5 : (!fir.ref<i8>) -> !fir.ref<f32>
- %7 = fircg.ext_declare %6 {uniq_name = "_QFEv2"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc20)
+ %7 = fircg.ext_declare %6 storage(%0[4]) {uniq_name = "_QFEv2"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc20)
%8 = fir.coordinate_of %1, %c8 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%9 = fir.convert %8 : (!fir.ref<i8>) -> !fir.ref<f32>
- %10 = fircg.ext_declare %9 {uniq_name = "_QFEv3"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc21)
+ %10 = fircg.ext_declare %9 storage(%0[8]) {uniq_name = "_QFEv3"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc21)
%11 = fir.address_of(@a_) : !fir.ref<tuple<i32, !fir.array<8xi8>>>
%12 = fir.convert %11 : (!fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<!fir.array<?xi8>>
%13 = fir.coordinate_of %12, %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%14 = fir.convert %13 : (!fir.ref<i8>) -> !fir.ref<f32>
- %15 = fircg.ext_declare %14 {uniq_name = "_QFEva1"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc22)
+ %15 = fircg.ext_declare %14 storage(%11[0]) {uniq_name = "_QFEva1"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc22)
%16 = fir.coordinate_of %12, %c4 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%17 = fir.convert %16 : (!fir.ref<i8>) -> !fir.ref<f32>
- %18 = fircg.ext_declare %17 {uniq_name = "_QFEva2"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc23)
+ %18 = fircg.ext_declare %17 storage(%11[4]) {uniq_name = "_QFEva2"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc23)
%19 = fir.coordinate_of %12, %c8 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%20 = fir.convert %19 : (!fir.ref<i8>) -> !fir.ref<f32>
- %21 = fircg.ext_declare %20 {uniq_name = "_QFEva3"} : (!fir.ref<f32>) -> !fir.ref<f32> loc(#loc24)
+ %21 = fircg.ext_declare %20 storage(%11[8]) {uniq_name = "_QFEva3"} : (!fir.ref<f32>, !fir.ref<tuple<i32, !fir.array<8xi8>>>) -> !fir.ref<f32> loc(#loc24)
return
} loc(#loc18)
}
|
vzakhari
approved these changes
Nov 19, 2025
Contributor
vzakhari
left a comment
vzakhari
left a comment
There was a problem hiding this comment.
LGTM! Thank you, Abid!
One minor comment: can you please add a line of description for fircg_XDeclareOp saying that most of the operands are inherited from fir_DeclareOp except for the shape and shift operands that are "expanded" forms of the corresponding shape/shift operands of fir_DeclareOp?
vzakhari
reviewed
Nov 19, 2025
| if (!global) | ||
| return false; | ||
|
|
||
| // Check if the global is actually a common block by demangling its name. |
Contributor
There was a problem hiding this comment.
Did the previous commit break any existing LIT tests? If not, can you please add one for regression testing?
Contributor
Author
There was a problem hiding this comment.
Exiting tests are fine. It just occurred to me that I should test it with module equivalance and I saw that those variables were also being marked as common block so I added this check to prevent this. I will add a test.
Contributor
Author
There was a problem hiding this comment.
I have added a test. Please let me know if you have any other comments.
Contributor
There was a problem hiding this comment.
Thank you, Abid! It looks great.
Our current implementation for extracting information about common block required traversal of FIR which was not ideal but previously there was no other way to obtain that information. The `[hl]fir.declare` was extended in commit llvm#155325 to include storage and storage_offset. This commit adds these operands in `fircg.ext_declare` and then use them in `AddDebugInfoPass` to create debug data for common blocks.
abidh
force-pushed
the
redo_common_blk
branch
from
ffed4d2 to
3160b07
Compare
🐧 Linux x64 Test Results
|
aadeshps-mcw
pushed a commit
to aadeshps-mcw/llvm-project
that referenced
this pull request
Nov 26, 2025
…168752) Our current implementation for extracting information about common block required traversal of FIR which was not ideal but previously there was no other way to obtain that information. The `[hl]fir.declare` was extended in commit llvm#155325 to include storage and storage_offset. This commit adds these operands in `fircg.ext_declare` and then use them in `AddDebugInfoPass` to create debug data for common blocks.
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Nov 26, 2025
…168752) Our current implementation for extracting information about common block required traversal of FIR which was not ideal but previously there was no other way to obtain that information. The `[hl]fir.declare` was extended in commit llvm#155325 to include storage and storage_offset. This commit adds these operands in `fircg.ext_declare` and then use them in `AddDebugInfoPass` to create debug data for common blocks.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Our current implementation for extracting information about common block required traversal of FIR which was not ideal but previously there was no other way to obtain that information. The
[hl]fir.declarewas extended in commit #155325 to include storage and storage_offset. This commit adds these operands infircg.ext_declareand then use them inAddDebugInfoPassto create debug data for common blocks.