Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][debug] Don't confuse count with upperBound. #98174

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Jul 9, 2024

The code that handled allocatable array had swapped count with upperBound. This did not get caught earlier as all the example were using 1 as lowerBound.

Fixes #98166.

With the fix in place, the GDB now correctly handles the case pointed in the bug ticket.
(gdb) p min::alloc2d
$2 = ((0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0))
(gdb) ptype min::alloc2d
type = integer, allocatable (-1:1,-2:2)

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jul 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2024

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

Author: Abid Qadeer (abidh)

Changes

The code that handled allocatable array had swapped count with upperBound. This did not get caught earlier as all the example were using 1 as lowerBound.

Fixes #98166.

With the fix in place, the GDB now correctly handles the case pointed in the bug ticket.
(gdb) p min::alloc2d
$2 = ((0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0))
(gdb) ptype min::alloc2d
type = integer, allocatable (-1:1,-2:2)


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

4 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+4-2)
  • (modified) flang/test/Integration/debug-assumed-shape-array.f90 (+2-2)
  • (modified) flang/test/Transforms/debug-assumed-shape-array.fir (+2-2)
  • (modified) flang/test/Transforms/debug-ptr-type.fir (+2-2)
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index 71cfee6d735ae..a56d3569418c6 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -143,7 +143,8 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
 
     offset += dimsSize;
     mlir::LLVM::DISubrangeAttr subrangeTy = mlir::LLVM::DISubrangeAttr::get(
-        context, nullptr, lowerAttr, countAttr, nullptr);
+        context, countAttr, lowerAttr, /*upperBound=*/nullptr,
+        /*stride=*/nullptr);
     elements.push_back(subrangeTy);
   }
   return mlir::LLVM::DICompositeTypeAttr::get(
@@ -177,7 +178,8 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
     auto countAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, dim));
     auto lowerAttr = mlir::IntegerAttr::get(intTy, llvm::APInt(64, 1));
     auto subrangeTy = mlir::LLVM::DISubrangeAttr::get(
-        context, countAttr, lowerAttr, nullptr, nullptr);
+        context, countAttr, lowerAttr, /*upperBound=*/nullptr,
+        /*stride=*/nullptr);
     elements.push_back(subrangeTy);
   }
   // Apart from arrays, the `DICompositeTypeAttr` is used for other things like
diff --git a/flang/test/Integration/debug-assumed-shape-array.f90 b/flang/test/Integration/debug-assumed-shape-array.f90
index 7b0801c12dba1..a23ffa13cf0ac 100644
--- a/flang/test/Integration/debug-assumed-shape-array.f90
+++ b/flang/test/Integration/debug-assumed-shape-array.f90
@@ -8,6 +8,6 @@ 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: ![[ELEMS]] = !{![[ELEM1:[0-9]+]], ![[ELEM2:[0-9]+]]}
-! CHECK-DAG: ![[ELEM1]] = !DISubrange(lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 24, DW_OP_deref), upperBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 32, DW_OP_deref))
-! CHECK-DAG: ![[ELEM2]] = !DISubrange(lowerBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 48, DW_OP_deref), upperBound: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 56, DW_OP_deref))
+! 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))
+! 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))
 
diff --git a/flang/test/Transforms/debug-assumed-shape-array.fir b/flang/test/Transforms/debug-assumed-shape-array.fir
index 00dec9b318c81..0a9b84ad253aa 100644
--- a/flang/test/Transforms/debug-assumed-shape-array.fir
+++ b/flang/test/Transforms/debug-assumed-shape-array.fir
@@ -11,6 +11,6 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i64, dense<64> :
 #loc2 = loc("test1.f90":3:16)
 
 // CHECK: #llvm.di_composite_type<tag = DW_TAG_array_type
-// CHECK-SAME: elements = #llvm.di_subrange<lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(24), DW_OP_deref]>, upperBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(32), DW_OP_deref]>>
-// CHECK-SAME: #llvm.di_subrange<lowerBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(48), DW_OP_deref]>, upperBound = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(56), DW_OP_deref]>>
+// 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]>>,
+// 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]>>
 // CHECK-SAME: dataLocation = <[DW_OP_push_object_address, DW_OP_deref]>>
diff --git a/flang/test/Transforms/debug-ptr-type.fir b/flang/test/Transforms/debug-ptr-type.fir
index 3f6c895ddaf8e..64e64cb1a19ae 100644
--- a/flang/test/Transforms/debug-ptr-type.fir
+++ b/flang/test/Transforms/debug-ptr-type.fir
@@ -32,8 +32,8 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
 #loc4 = loc("test.f90":8:1)
 
 // CHECK-DAG: #[[INT_TY:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "integer"{{.*}}>
-// CHECK-DAG: #[[ARR1_TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<lowerBound = #llvm.di_expression{{.*}}, upperBound = #llvm.di_expression{{.*}}>, #llvm.di_subrange<lowerBound = #llvm.di_expression{{.*}}, upperBound = #llvm.di_expression{{.*}}>, dataLocation = {{.*}}, associated = <[DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne]>>
-// CHECK-DAG: #[[ARR2_TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<lowerBound = #llvm.di_expression{{.*}}, upperBound = #llvm.di_expression{{.*}}>, dataLocation = {{.*}}, associated = <[DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne]>>
+// CHECK-DAG: #[[ARR1_TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<count = #llvm.di_expression<{{.*}}>, lowerBound = #llvm.di_expression<{{.*}}>, #llvm.di_subrange<count = #llvm.di_expression<{{.*}}>, lowerBound = #llvm.di_expression<{{.*}}>>, dataLocation = {{.*}}, associated = <[DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne]>>
+// CHECK-DAG: #[[ARR2_TY:.*]] = #llvm.di_composite_type<tag = DW_TAG_array_type{{.*}}elements = #llvm.di_subrange<count = #llvm.di_expression<{{.*}}>, lowerBound = #llvm.di_expression<{{.*}}>>, dataLocation = {{.*}}, associated = <[DW_OP_push_object_address, DW_OP_deref, DW_OP_lit0, DW_OP_ne]>>
 // CHECK-DAG: #[[PTR_TY:.*]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type{{.*}}baseType = #[[INT_TY]]{{.*}}>
 // CHECK-DAG: #llvm.di_global_variable<{{.*}}name = "par"{{.*}}type = #[[ARR1_TY]]{{.*}}>
 // CHECK-DAG: #llvm.di_global_variable<{{.*}}name = "par2"{{.*}}type = #[[ARR2_TY]]{{.*}}>

Copy link
Contributor

@jeanPerier jeanPerier 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 requested a review from jinisusan July 10, 2024 09:15
The code that handled allocatable array had swapped `count` with
`upperBound`. This did not get caught earlier as all the example were
using 1 as `lowerBound`.

Fixes llvm#98166.
Copy link
Contributor

@tblah tblah 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 51fac77 into llvm:main Jul 10, 2024
7 checks passed
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
The code that handled allocatable array had swapped `count` with
`upperBound`. This did not get caught earlier as all the example were
using 1 as `lowerBound`.

Fixes llvm#98166.

With the fix in place, the GDB now correctly handles the case pointed in
the bug ticket.
(gdb) p min::alloc2d
$2 = ((0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0) (0, 0, 0))
(gdb) ptype min::alloc2d
type = integer, allocatable (-1:1,-2:2)
@abidh abidh deleted the 98166 branch July 17, 2024 09:56
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.

[flang][debug] Bound of allocatable array are wrong.
4 participants