[flang][debug] Better handle array lower bound of assumed shape arrays. #110302
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
abidh
requested review from
tblah,
jeanPerier,
dpalermo,
pawosm-arm,
jinisusan and
kiranktp
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Abid Qadeer (abidh) ChangesAs mentioned in #108633, we don't respect the lower bound of the assumed shape arrays if those were specified. It happens in both cases:
This PR tries to fix this issue by improving our generation of lower bound attribute on DICompositeTypeAttr. If we see a lower bound in the declaration, we respect that. Note that same function is also used for allocatable/pointer variables. We make sure that we get the lower bound from descriptor in those cases. Please note that DWARF assumes a lower bound of 1 so in many cases we don't need to generate the lower bound. Fixes #108633. Full diff: https://github.com/llvm/llvm-project/pull/110302.diff 5 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 4aa14ca2c2bdd6..d498e838e9138a 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -114,10 +114,19 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
mlir::LLVM::DITypeAttr elemTy =
convertType(seqTy.getEleTy(), fileAttr, scope, declOp);
unsigned offset = dimsOffset;
+ unsigned index = 0;
+ mlir::IntegerType intTy = mlir::IntegerType::get(context, 64);
const unsigned indexSize = dimsSize / 3;
for ([[maybe_unused]] auto _ : seqTy.getShape()) {
// For each dimension, find the offset of count, lower bound and stride in
// the descriptor and generate the dwarf expression to extract it.
+ mlir::Attribute lowerAttr = nullptr;
+ // If declaration has a lower bound, use it.
+ if (declOp && declOp.getShift().size() > index) {
+ if (std::optional<std::int64_t> optint =
+ getIntIfConstant(declOp.getShift()[index]))
+ lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, *optint));
+ }
// FIXME: If `indexSize` happens to be bigger than address size on the
// system then we may have to change 'DW_OP_deref' here.
addOp(llvm::dwarf::DW_OP_push_object_address, {});
@@ -130,14 +139,19 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
mlir::LLVM::DIExpressionAttr::get(context, ops);
ops.clear();
- addOp(llvm::dwarf::DW_OP_push_object_address, {});
- addOp(llvm::dwarf::DW_OP_plus_uconst,
- {offset + (indexSize * kDimLowerBoundPos)});
- addOp(llvm::dwarf::DW_OP_deref, {});
- // lower_bound[i] = *(base_addr + offset + (indexSize * kDimLowerBoundPos))
- mlir::LLVM::DIExpressionAttr lowerAttr =
- mlir::LLVM::DIExpressionAttr::get(context, ops);
- ops.clear();
+ // If a lower bound was not found in the declOp, then we will get them from
+ // descriptor only for pointer and allocatable case. DWARF assumes lower
+ // bound of 1 when this attribute is missing.
+ if (!lowerAttr && (genAllocated || genAssociated)) {
+ addOp(llvm::dwarf::DW_OP_push_object_address, {});
+ addOp(llvm::dwarf::DW_OP_plus_uconst,
+ {offset + (indexSize * kDimLowerBoundPos)});
+ addOp(llvm::dwarf::DW_OP_deref, {});
+ // lower_bound[i] = *(base_addr + offset + (indexSize *
+ // kDimLowerBoundPos))
+ lowerAttr = mlir::LLVM::DIExpressionAttr::get(context, ops);
+ ops.clear();
+ }
addOp(llvm::dwarf::DW_OP_push_object_address, {});
addOp(llvm::dwarf::DW_OP_plus_uconst,
@@ -152,6 +166,7 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
mlir::LLVM::DISubrangeAttr subrangeTy = mlir::LLVM::DISubrangeAttr::get(
context, countAttr, lowerAttr, /*upperBound=*/nullptr, strideAttr);
elements.push_back(subrangeTy);
+ ++index;
}
return mlir::LLVM::DICompositeTypeAttr::get(
context, llvm::dwarf::DW_TAG_array_type, /*name=*/nullptr,
diff --git a/flang/test/Integration/debug-allocatable-1.f90 b/flang/test/Integration/debug-allocatable-1.f90
index 471c8cdb7d54eb..b9de3b26cdf984 100644
--- a/flang/test/Integration/debug-allocatable-1.f90
+++ b/flang/test/Integration/debug-allocatable-1.f90
@@ -17,8 +17,8 @@ end subroutine ff
! CHECK-DAG: !DILocalVariable(name: "ar1"{{.*}}type: ![[TY1:[0-9]+]])
! CHECK-DAG: ![[TY1]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS2:[0-9]+]]{{.*}}dataLocation{{.*}}allocated: !DIExpression(DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne))
! CHECK-DAG: ![[ELEMS2]] = !{![[ELEM1:[0-9]+]], ![[ELEM2:[0-9]+]]}
-! CHECK-DAG: ![[ELEM1]] = !DISubrange
-! CHECK-DAG: ![[ELEM2]] = !DISubrange
+! CHECK-DAG: ![[ELEM1]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref))
+! CHECK-DAG: ![[ELEM2]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref))
! CHECK-DAG: !DILocalVariable(name: "sc"{{.*}}type: ![[TY2:[0-9]+]])
! CHECK-DAG: ![[TY2]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[TY3:[0-9]+]]{{.*}})
! CHECK-DAG: ![[TY3]] = !DIBasicType(name: "real"{{.*}})
diff --git a/flang/test/Integration/debug-assumed-shape-array.f90 b/flang/test/Integration/debug-assumed-shape-array.f90
index 9a439e20d19816..38a8fd7880f53c 100644
--- a/flang/test/Integration/debug-assumed-shape-array.f90
+++ b/flang/test/Integration/debug-assumed-shape-array.f90
@@ -1,13 +1,20 @@
! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
-subroutine ff(arr)
+subroutine ff(arr, arr1)
implicit none
integer :: arr(:, :)
- return arr(1,1)
+ integer :: arr1(3:, 4:)
+ return arr(1,1) + arr1(3,4)
end subroutine ff
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS:[0-9]+]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref))
+! CHECK-DAG: !DILocalVariable(name: "arr"{{.*}}type: ![[TY1:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[TY1]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS:[0-9]+]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref))
! CHECK-DAG: ![[ELEMS]] = !{![[ELEM1:[0-9]+]], ![[ELEM2:[0-9]+]]}
-! CHECK-DAG: ![[ELEM1]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 32, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 24, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 40, DW_OP_deref))
-! CHECK-DAG: ![[ELEM2]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 56, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 48, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 64, DW_OP_deref))
+! CHECK-DAG: ![[ELEM1]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 32, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 40, DW_OP_deref))
+! CHECK-DAG: ![[ELEM2]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 56, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 64, DW_OP_deref))
+! CHECK-DAG: !DILocalVariable(name: "arr1"{{.*}}type: ![[TY2:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[TY2]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS1:[0-9]+]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref))
+! CHECK-DAG: ![[ELEMS1]] = !{![[ELEM11:[0-9]+]], ![[ELEM12:[0-9]+]]}
+! CHECK-DAG: ![[ELEM11]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 32, DW_OP_deref), lowerBound: 3, stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 40, DW_OP_deref))
+! CHECK-DAG: ![[ELEM12]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 56, DW_OP_deref), lowerBound: 4, stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 64, DW_OP_deref))
\ No newline at end of file
diff --git a/flang/test/Integration/debug-ptr-type.f90 b/flang/test/Integration/debug-ptr-type.f90
index bff7bcb862b5c3..6d7178f7aca41f 100644
--- a/flang/test/Integration/debug-ptr-type.f90
+++ b/flang/test/Integration/debug-ptr-type.f90
@@ -41,7 +41,9 @@ end subroutine ff
! CHECK-DAG: ![[ELEMS1:[0-9]+]] = !{!{{[0-9]+}}}
! CHECK-DAG: !DILocalVariable(name: "par"{{.*}}type: ![[ARR_TY1:[0-9]+]])
! CHECK-DAG: ![[ARR_TY1]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}elements: ![[ELEMS2:[0-9]+]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref), associated: !DIExpression(DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne))
-! CHECK-DAG: ![[ELEMS2]] = !{!{{[0-9]+}}, !{{[0-9]+}}}
+! CHECK-DAG: ![[ELEMS2]] = !{![[ELEM21:[0-9]+]], ![[ELEM22:[0-9]+]]}
+! CHECK-DAG: ![[ELEM21]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref))
+! CHECK-DAG: ![[ELEM22]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), stride: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref))
! CHECK-DAG: !DILocalVariable(name: "par2"{{.*}}type: ![[ARR_TY2:[0-9]+]])
! CHECK-DAG: ![[ARR_TY2]] = !DICompositeType(tag: DW_TAG_array_type{{.*}}, elements: ![[ELEMS1]], dataLocation: !DIExpression(DW_OP_push_object_address, DW_OP_deref), associated: !DIExpression(DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne))
! CHECK-DAG: !DILocalVariable(name: "psc"{{.*}}type: ![[PTR_TY:[0-9]+]])
diff --git a/flang/test/Transforms/debug-assumed-shape-array.fir b/flang/test/Transforms/debug-assumed-shape-array.fir
index d1e64297acea7d..cb3927a7d79cf3 100644
--- a/flang/test/Transforms/debug-assumed-shape-array.fir
+++ b/flang/test/Transforms/debug-assumed-shape-array.fir
@@ -1,16 +1,25 @@
// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", 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"} {
- func.func @ff_(%arg0: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "arr"} ) {
+ func.func @ff_(%arg0: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "arr"}, %arg1: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "arr1"}) {
+ %c4 = arith.constant 4 : index
+ %c3 = arith.constant 3 : index
%0 = fir.undefined !fir.dscope
%1 = fircg.ext_declare %arg0 dummy_scope %0 {uniq_name = "_QFffEarr"} : (!fir.box<!fir.array<?x?xi32>>, !fir.dscope) -> !fir.box<!fir.array<?x?xi32>> loc(#loc1)
+ %2 = fircg.ext_declare %arg1 origin %c3, %c4 dummy_scope %0 {uniq_name = "_QFffEarr1"} : (!fir.box<!fir.array<?x?xi32>>, index, index, !fir.dscope) -> !fir.box<!fir.array<?x?xi32>> loc(#loc3)
return
} loc(#loc2)
}
#loc1 = loc("test1.f90":1:1)
#loc2 = loc("test1.f90":3:16)
+#loc3 = loc("test1.f90":4:16)
-// CHECK: #llvm.di_composite_type<tag = DW_TAG_array_type
-// CHECK-SAME: elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(32), DW_OP_deref]>, lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(24), DW_OP_deref]>, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(40), DW_OP_deref]>>,
-// CHECK-SAME: #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(56), DW_OP_deref]>, lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(48), DW_OP_deref]>, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(64), DW_OP_deref]>>
+// CHECK: #[[TY1:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type
+// CHECK-SAME: elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(32), DW_OP_deref]>, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(40), DW_OP_deref]>>,
+// CHECK-SAME: #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(56), DW_OP_deref]>, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(64), DW_OP_deref]>>
// CHECK-SAME: dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>>
+// CHECK: #[[TY2:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type
+// CHECK-SAME: elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(32), DW_OP_deref]>, lowerBound = 3 : i64, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(40), DW_OP_deref]>>,
+// CHECK-SAME: #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(56), DW_OP_deref]>, lowerBound = 4 : i64, stride = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(64), DW_OP_deref]>>, dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>>
+// CHECK: #llvm.di_local_variable<{{.*}}name = "arr"{{.*}}type = #[[TY1]]>
+// CHECK: #llvm.di_local_variable<{{.*}}name = "arr1"{{.*}}type = #[[TY2]]>
|
pawosm-arm
reviewed
Sep 29, 2024
pawosm-arm
approved these changes
Sep 29, 2024
As mentioned in llvm#108633, we don't respect the lower bound of the assumed shape arrays if those were specified. It happens in both cases: 1. When caller has non-default lower bound and callee has default 2. When callee has non-default lower bound and caller has default This PR tries to fix this issue by improving our generation of lower bound attribute on DICompositeTypeAttr. If we see a lower bound in the declaration, we respect that. Note that same function is also used for allocatable/pointer variables. We make sure that we get the lower bound from descriptor in those cases. Please note that DWARF assumes a lower bound of 1 so in many cases we don't need to generate the lower bound. Fixes llvm#108633.
jeanPerier
approved these changes
Sep 30, 2024
| if (declOp && declOp.getShift().size() > index) { | ||
| if (std::optional<std::int64_t> optint = | ||
| getIntIfConstant(declOp.getShift()[index])) | ||
| lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, *optint)); |
There was a problem hiding this comment.
Is it possible to leave a TODO note for the non constant case to do like for non constant character length? I do not think falling into reading the input descriptor lower bounds is correct.
subroutine foo(x, n)
integer :: n
real :: x(n:)
end subroutine
There was a problem hiding this comment.
Thanks for the review. Indeed the variable lower bound case is not handled and I have added a TODO.
Add a TODO comment that we still need to handle variable lower bound.
abidh
force-pushed
the
lower_bound_assumed_shape
branch
from
b08bc7e to
06e98f1
Compare
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
…s. (llvm#110302) As mentioned in llvm#108633, we don't respect the lower bound of the assumed shape arrays if those were specified. It happens in both cases: 1. When caller has non-default lower bound and callee has default 2. When callee has non-default lower bound and caller has default This PR tries to fix this issue by improving our generation of lower bound attribute on DICompositeTypeAttr. If we see a lower bound in the declaration, we respect that. Note that same function is also used for allocatable/pointer variables. We make sure that we get the lower bound from descriptor in those cases. Please note that DWARF assumes a lower bound of 1 so in many cases we don't need to generate the lower bound. Fixes llvm#108633.
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.
As mentioned in #108633, we don't respect the lower bound of the assumed shape arrays if those were specified. It happens in both cases:
This PR tries to fix this issue by improving our generation of lower bound attribute on DICompositeTypeAttr. If we see a lower bound in the declaration, we respect that. Note that same function is also used for allocatable/pointer variables. We make sure that we get the lower bound from descriptor in those cases. Please note that DWARF assumes a lower bound of 1 so in many cases we don't need to generate the lower bound.
Fixes #108633.