Skip to content

[flang][debug] Support TupleType. #113917

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

Merged
merged 1 commit into from
Oct 30, 2024
Merged

[flang][debug] Support TupleType. #113917

merged 1 commit into from
Oct 30, 2024

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Oct 28, 2024

Handling is similar to RecordType with following differences:

  1. No check for cyclic references
  2. No extra processing for lower bounds of array members.
  3. No line information as TupleType is a lowering artefact and does not really represent an entity in the code.

Handling is similar to RecordType.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Oct 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 28, 2024

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

Author: Abid Qadeer (abidh)

Changes

Handling is similar to RecordType.


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

3 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp (+52-9)
  • (modified) flang/lib/Optimizer/Transforms/DebugTypeGenerator.h (+6)
  • (added) flang/test/Transforms/debug-tuple-type.fir (+15)
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
index e387e27533a006..0337bfa573fb3c 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
@@ -271,6 +271,19 @@ static bool canCacheThisType(mlir::LLVM::DICompositeTypeAttr comTy) {
   return true;
 }
 
+std::pair<std::uint64_t, unsigned short>
+DebugTypeGenerator::getFieldSizeAndAlign(mlir::Type fieldTy) {
+  mlir::Type llvmTy;
+  if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(fieldTy))
+    llvmTy = llvmTypeConverter.convertBoxTypeAsStruct(boxTy, getBoxRank(boxTy));
+  else
+    llvmTy = llvmTypeConverter.convertType(fieldTy);
+
+  uint64_t byteSize = dataLayout->getTypeSize(llvmTy);
+  unsigned short byteAlign = dataLayout->getTypeABIAlignment(llvmTy);
+  return std::pair{byteSize, byteAlign};
+}
+
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     fir::RecordType Ty, mlir::LLVM::DIFileAttr fileAttr,
     mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) {
@@ -303,15 +316,7 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
   mlir::IntegerType intTy = mlir::IntegerType::get(context, 64);
   std::uint64_t offset = 0;
   for (auto [fieldName, fieldTy] : Ty.getTypeList()) {
-    mlir::Type llvmTy;
-    if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(fieldTy))
-      llvmTy =
-          llvmTypeConverter.convertBoxTypeAsStruct(boxTy, getBoxRank(boxTy));
-    else
-      llvmTy = llvmTypeConverter.convertType(fieldTy);
-
-    uint64_t byteSize = dataLayout->getTypeSize(llvmTy);
-    unsigned short byteAlign = dataLayout->getTypeABIAlignment(llvmTy);
+    auto [byteSize, byteAlign] = getFieldSizeAndAlign(fieldTy);
     std::optional<llvm::ArrayRef<int64_t>> lowerBounds =
         fir::getComponentLowerBoundsIfNonDefault(Ty, fieldName, module,
                                                  symbolTable);
@@ -368,6 +373,42 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
   return finalAttr;
 }
 
+mlir::LLVM::DITypeAttr DebugTypeGenerator::convertTupleType(
+    mlir::TupleType Ty, mlir::LLVM::DIFileAttr fileAttr,
+    mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) {
+  // Check if this type has already been converted.
+  auto iter = typeCache.find(Ty);
+  if (iter != typeCache.end())
+    return iter->second;
+
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
+  mlir::MLIRContext *context = module.getContext();
+
+  std::uint64_t offset = 0;
+  for (auto fieldTy : Ty.getTypes()) {
+    auto [byteSize, byteAlign] = getFieldSizeAndAlign(fieldTy);
+    mlir::LLVM::DITypeAttr elemTy =
+        convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
+    offset = llvm::alignTo(offset, byteAlign);
+    mlir::LLVM::DIDerivedTypeAttr tyAttr = mlir::LLVM::DIDerivedTypeAttr::get(
+        context, llvm::dwarf::DW_TAG_member, mlir::StringAttr::get(context, ""),
+        elemTy, byteSize * 8, byteAlign * 8, offset * 8,
+        /*optional<address space>=*/std::nullopt,
+        /*extra data=*/nullptr);
+    elements.push_back(tyAttr);
+    offset += llvm::alignTo(byteSize, byteAlign);
+  }
+
+  auto typeAttr = mlir::LLVM::DICompositeTypeAttr::get(
+      context, llvm::dwarf::DW_TAG_structure_type,
+      mlir::StringAttr::get(context, ""), fileAttr, /*line=*/0, scope,
+      /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, offset * 8,
+      /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
+      /*allocated=*/nullptr, /*associated=*/nullptr);
+  typeCache[Ty] = typeAttr;
+  return typeAttr;
+}
+
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(
     fir::SequenceType seqTy, mlir::LLVM::DIFileAttr fileAttr,
     mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) {
@@ -574,6 +615,8 @@ DebugTypeGenerator::convertType(mlir::Type Ty, mlir::LLVM::DIFileAttr fileAttr,
                                 /*hasDescriptor=*/false);
   } else if (auto recTy = mlir::dyn_cast_or_null<fir::RecordType>(Ty)) {
     return convertRecordType(recTy, fileAttr, scope, declOp);
+  } else if (auto tupleTy = mlir::dyn_cast_if_present<mlir::TupleType>(Ty)) {
+    return convertTupleType(tupleTy, fileAttr, scope, declOp);
   } else if (auto refTy = mlir::dyn_cast_if_present<fir::ReferenceType>(Ty)) {
     auto elTy = refTy.getEleTy();
     return convertPointerLikeType(elTy, fileAttr, scope, declOp,
diff --git a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
index eeefb6c463d936..c1fce4bdae5ce5 100644
--- a/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
+++ b/flang/lib/Optimizer/Transforms/DebugTypeGenerator.h
@@ -39,6 +39,10 @@ class DebugTypeGenerator {
                                            mlir::LLVM::DIFileAttr fileAttr,
                                            mlir::LLVM::DIScopeAttr scope,
                                            fir::cg::XDeclareOp declOp);
+  mlir::LLVM::DITypeAttr convertTupleType(mlir::TupleType Ty,
+                                          mlir::LLVM::DIFileAttr fileAttr,
+                                          mlir::LLVM::DIScopeAttr scope,
+                                          fir::cg::XDeclareOp declOp);
   mlir::LLVM::DITypeAttr convertSequenceType(fir::SequenceType seqTy,
                                              mlir::LLVM::DIFileAttr fileAttr,
                                              mlir::LLVM::DIScopeAttr scope,
@@ -73,6 +77,8 @@ class DebugTypeGenerator {
                              mlir::LLVM::DIFileAttr fileAttr,
                              mlir::LLVM::DIScopeAttr scope,
                              fir::cg::XDeclareOp declOp);
+  std::pair<std::uint64_t, unsigned short>
+  getFieldSizeAndAlign(mlir::Type fieldTy);
 
   mlir::ModuleOp module;
   mlir::SymbolTable *symbolTable;
diff --git a/flang/test/Transforms/debug-tuple-type.fir b/flang/test/Transforms/debug-tuple-type.fir
new file mode 100644
index 00000000000000..c9b0d16c06e1ae
--- /dev/null
+++ b/flang/test/Transforms/debug-tuple-type.fir
@@ -0,0 +1,15 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+  func.func private @fn1(!fir.ref<tuple<f64, f64>>)
+  func.func private @_FortranAioOutputDerivedType(!fir.ref<tuple<>>)
+}
+
+// CHECK: #[[F64:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "real", sizeInBits = 64, encoding = DW_ATE_float>
+// CHECK: #[[CU:.*]] = #llvm.di_compile_unit<{{.*}}>
+// CHECK: #[[DTY1:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "", baseType = #[[F64]], sizeInBits = 64, alignInBits = {{.*}}>
+// CHECK: #[[DTY2:.*]] = #llvm.di_derived_type<tag = DW_TAG_member, name = "", baseType = #[[F64]], sizeInBits = 64, alignInBits = {{.*}}, offsetInBits = {{.*}}>
+// CHECK: #[[COM_TY1:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "", file = #{{.*}}, scope = #[[CU]]{{.*}}elements = #[[DTY1]], #[[DTY2]]>
+// CHECK: #[[COM_TY2:.*]] = #llvm.di_composite_type<tag = DW_TAG_structure_type, name = "", file = #{{.*}}, scope = #[[CU]]>
+// CHECK: #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #di_null_type, #[[COM_TY1]]>
+// CHECK: #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #di_null_type, #[[COM_TY2]]>

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.

One comment, otherwise this looks good to me.


auto typeAttr = mlir::LLVM::DICompositeTypeAttr::get(
context, llvm::dwarf::DW_TAG_structure_type,
mlir::StringAttr::get(context, ""), fileAttr, /*line=*/0, scope,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking I understand this correctly (maybe some equivalent information belongs in the commit message?):

We can't give a line number here because TupleType doesn't really correspond to a variable defined in fortran source. It is just used by lowering in a few places so it can make it into IR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct understanding. I will add a comment here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the description.

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.

These tuple types are used in three contexts:

  • in the runtime to implement C struct like types (IO cookie, derived type bindings...)
  • to implement the host link of internal procedures (I actually do not know if DWARF needs to know something specific here).
  • to implement target ABIs for BIND(C) types and COMPLEX passed in registers.

The last case actually means that the type of functions returning complex may actually not match the dwarf types generated by other compilers for:

complex(8) function foo(z) bind(c)
  complex(8), value :: z
  foo = z
end function

classic flang generates:

!11 = !DIBasicType(tag: DW_TAG_base_type, name: "double complex", size: 128, align: 64, encoding: DW_ATE_complex_float)
!12 = !{ !11, !11 }
!13 = !DISubroutineType(types: !12)

And flang will generate:

!5 = !DISubroutineType(cc: DW_CC_normal, types: !6)
!6 = !{!7, !10, !10}
!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !1, file: !2, size: 128, elements: !8)
!8 = !{!9, !11}
!9 = !DIDerivedType(tag: DW_TAG_member, baseType: !10, size: 64, align: 64)
!10 = !DIBasicType(name: "real", size: 64, encoding: DW_ATE_float)
!11 = !DIDerivedType(tag: DW_TAG_member, baseType: !10, size: 64, align: 64, offset: 64)

We likely need a solution to keep the dwarf at the Fortran level here.
Maybe the debug info pass should be run before the target rewrite pass. This can be tested in another patch though.

@abidh
Copy link
Contributor Author

abidh commented Oct 29, 2024

We likely need a solution to keep the dwarf at the Fortran level here.
Maybe the debug info pass should be run before the target rewrite pass. This can be tested in another patch though.

Thanks for the detailed reply. I have also been thinking about running AddDebugInfo pass early because of the issue you mentioned. Note issue #108711 and similar issue for derived type #112902 that I opened sometime ago.

Do I understand you right that we should merge this and handle the issue with running AddDebugInfo in a separate PR?

@abidh abidh merged commit 652988b into llvm:main Oct 30, 2024
11 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Handling is similar to RecordType with following differences:

1. No check for cyclic references
2. No extra processing for lower bounds of array members.
3. No line information as TupleType is a lowering artefact and does not
really represent an entity in the code.
abidh added a commit that referenced this pull request Nov 5, 2024
This help us generate debug info that better represents the actual
Fortran source code. It was briefly discussed
[here](#113917 (review)).
    
Fixes #108711.
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
This help us generate debug info that better represents the actual
Fortran source code. It was briefly discussed
[here](llvm#113917 (review)).
    
Fixes llvm#108711.
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.

4 participants