[flang][debug] Support derived type components with box types. (#109424)

Our support for derived types uses `getTypeSizeAndAlignment` to
calculate the offset of the members. The `fir.box` was not supported in
that function. It meant that any member which required descriptor was
not supported in the derived type.
    
We convert the type into an llvm type and then use the DataLayout to
calculate the size/offset of a member. There is no dependency on
`getTypeSizeAndAlignment` to get the size of the types.

There are 2 other changes in this PR:

1. The `recID` field is used to handle cases where we have a member
references its parent type.

2. A type cache is maintained to avoid duplication. It is also needed
for circular reference case.


Fixes #108001.

Author: abidh

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

@@ -14,7 +14,6 @@
 
 #include "DebugTypeGenerator.h"
 #include "flang/Optimizer/CodeGen/DescriptorModel.h"
-#include "flang/Optimizer/CodeGen/TypeConverter.h"
 #include "flang/Optimizer/Support/InternalNames.h"
 #include "mlir/Pass/Pass.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -48,7 +47,7 @@ DebugTypeGenerator::DebugTypeGenerator(mlir::ModuleOp m,
                                        mlir::SymbolTable *symbolTable_,
                                        const mlir::DataLayout &dl)
     : module(m), symbolTable(symbolTable_), dataLayout{&dl},
-      kindMapping(getKindMapping(m)) {
+      kindMapping(getKindMapping(m)), llvmTypeConverter(m, false, false, dl) {
   LLVM_DEBUG(llvm::dbgs() << "DITypeAttr generator\n");
 
   mlir::MLIRContext *context = module.getContext();
@@ -160,29 +159,110 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
       dataLocation, /*rank=*/nullptr, allocated, associated);
 }
 
+// If the type is a pointer or array type then gets its underlying type.
+static mlir::LLVM::DITypeAttr getUnderlyingType(mlir::LLVM::DITypeAttr Ty) {
+  if (auto ptrTy =
+          mlir::dyn_cast_if_present<mlir::LLVM::DIDerivedTypeAttr>(Ty)) {
+    if (ptrTy.getTag() == llvm::dwarf::DW_TAG_pointer_type)
+      Ty = getUnderlyingType(ptrTy.getBaseType());
+  }
+  if (auto comTy =
+          mlir::dyn_cast_if_present<mlir::LLVM::DICompositeTypeAttr>(Ty)) {
+    if (comTy.getTag() == llvm::dwarf::DW_TAG_array_type)
+      Ty = getUnderlyingType(comTy.getBaseType());
+  }
+  return Ty;
+}
+
+// Currently, the handling of recursive debug type in mlir has some limitations.
+// Those limitations were discussed at the end of the thread for following PR.
+// https://github.com/llvm/llvm-project/pull/106571
+//
+// Problem could be explained with the following example code:
+//  type t2
+//   type(t1), pointer :: p1
+// end type
+// type t1
+//   type(t2), pointer :: p2
+// end type
+// In the description below, type_self means a temporary type that is generated
+// as a place holder while the members of that type are being processed.
+//
+// If we process t1 first then we will have the following structure after it has
+// been processed.
+// t1 -> t2 -> t1_self
+// This is because when we started processing t2, we did not have the complete
+// t1 but its place holder t1_self.
+// Now if some entity requires t2, we will already have that in cache and will
+// return it. But this t2 refers to t1_self and not to t1. In mlir handling,
+// only those types are allowed to have _self reference which are wrapped by
+// entity whose reference it is. So t1 -> t2 -> t1_self is ok because the
+// t1_self reference can be resolved by the outer t1. But standalone t2 is not
+// because there will be no way to resolve it. Until this is fixed in mlir, we
+// avoid caching such types. Please see DebugTranslation::translateRecursive for
+// details on how mlir handles recursive types.
+static bool canCacheThisType(mlir::LLVM::DICompositeTypeAttr comTy) {
+  for (auto el : comTy.getElements()) {
+    if (auto mem =
+            mlir::dyn_cast_if_present<mlir::LLVM::DIDerivedTypeAttr>(el)) {
+      mlir::LLVM::DITypeAttr memTy = getUnderlyingType(mem.getBaseType());
+      if (auto baseTy =
+              mlir::dyn_cast_if_present<mlir::LLVM::DICompositeTypeAttr>(
+                  memTy)) {
+        // We will not cache a type if one of its member meets the following
+        // conditions:
+        // 1. It is a structure type
+        // 2. It is a place holder type (getIsRecSelf() is true)
+        // 3. It is not a self reference. It is ok to have t1_self in t1.
+        if (baseTy.getTag() == llvm::dwarf::DW_TAG_structure_type &&
+            baseTy.getIsRecSelf() && (comTy.getRecId() != baseTy.getRecId()))
+          return false;
+      }
+    }
+  }
+  return true;
+}
+
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     fir::RecordType 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();
+  auto recId = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+  // Generate a place holder TypeAttr which will be used if a member
+  // references the parent type.
+  auto comAttr = mlir::LLVM::DICompositeTypeAttr::get(
+      context, recId, /*isRecSelf=*/true, llvm::dwarf::DW_TAG_structure_type,
+      mlir::StringAttr::get(context, ""), fileAttr, /*line=*/0, scope,
+      /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, /*sizeInBits=*/0,
+      /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
+      /*allocated=*/nullptr, /*associated=*/nullptr);
+  typeCache[Ty] = comAttr;
+
   auto result = fir::NameUniquer::deconstruct(Ty.getName());
   if (result.first != fir::NameUniquer::NameKind::DERIVED_TYPE)
     return genPlaceholderType(context);
 
   fir::TypeInfoOp tiOp = symbolTable->lookup<fir::TypeInfoOp>(Ty.getName());
   unsigned line = (tiOp) ? getLineFromLoc(tiOp.getLoc()) : 1;
 
-  llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
   std::uint64_t offset = 0;
   for (auto [fieldName, fieldTy] : Ty.getTypeList()) {
-    auto result = fir::getTypeSizeAndAlignment(module.getLoc(), fieldTy,
-                                               *dataLayout, kindMapping);
-    // If we get a type whose size we can't determine, we will break the loop
-    // and generate the derived type with whatever components we have
-    // assembled thus far.
-    if (!result)
-      break;
-    auto [byteSize, byteAlign] = *result;
+    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);
+
     // FIXME: Handle non defaults array bound in derived types
+    uint64_t byteSize = dataLayout->getTypeSize(llvmTy);
+    unsigned short byteAlign = dataLayout->getTypeABIAlignment(llvmTy);
     mlir::LLVM::DITypeAttr elemTy =
         convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
     offset = llvm::alignTo(offset, byteAlign);
@@ -195,12 +275,20 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     offset += llvm::alignTo(byteSize, byteAlign);
   }
 
-  return mlir::LLVM::DICompositeTypeAttr::get(
-      context, llvm::dwarf::DW_TAG_structure_type,
+  auto finalAttr = mlir::LLVM::DICompositeTypeAttr::get(
+      context, recId, /*isRecSelf=*/false, llvm::dwarf::DW_TAG_structure_type,
       mlir::StringAttr::get(context, result.second.name), fileAttr, line, scope,
       /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, offset * 8,
       /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
       /*allocated=*/nullptr, /*associated=*/nullptr);
+  if (canCacheThisType(finalAttr)) {
+    typeCache[Ty] = finalAttr;
+  } else {
+    auto iter = typeCache.find(Ty);
+    if (iter != typeCache.end())
+      typeCache.erase(iter);
+  }
+  return finalAttr;
 }
 
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(

flang/lib/Optimizer/Transforms/DebugTypeGenerator.h

@@ -14,6 +14,7 @@
 #define FORTRAN_OPTIMIZER_TRANSFORMS_DEBUGTYPEGENERATOR_H
 
 #include "flang/Optimizer/CodeGen/CGOps.h"
+#include "flang/Optimizer/CodeGen/TypeConverter.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Dialect/Support/FIRContext.h"
 #include "flang/Optimizer/Dialect/Support/KindMapping.h"
@@ -68,10 +69,12 @@ class DebugTypeGenerator {
   mlir::SymbolTable *symbolTable;
   const mlir::DataLayout *dataLayout;
   KindMapping kindMapping;
+  fir::LLVMTypeConverter llvmTypeConverter;
   std::uint64_t dimsSize;
   std::uint64_t dimsOffset;
   std::uint64_t ptrSize;
   std::uint64_t lenOffset;
+  llvm::DenseMap<mlir::Type, mlir::LLVM::DITypeAttr> typeCache;
 };
 
 } // namespace fir

flang/test/Integration/debug-cyclic-derived-type-2.f90

@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck  %s
+
+! mainly test that this program does not cause an assertion failure
+module m
+ type t2
+   type(t1), pointer :: p1
+ end type
+ type t1
+   type(t2), pointer :: p2
+   integer abc
+ end type
+ type(t1) :: tee1
+end module
+
+program test
+  use m
+  type(t2) :: lc2
+  print *, lc2%p1%abc
+end program test
+
+! CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}})
+! CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}})

flang/test/Integration/debug-cyclic-derived-type.f90

@@ -11,5 +11,11 @@ module m
  type(t2) :: v3
 end module
 
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}})
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}})
+! CHECK-DAG: ![[T1:[0-9]+]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}}elements: ![[T1_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T1_ELEMS]] = !{![[T1_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T1_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "p", baseType: ![[T2P:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[T2P]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[T2:[0-9]+]]{{.*}})
+
+! CHECK-DAG: ![[T2]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}}elements: ![[T2_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T2_ELEMS]] = !{![[T2_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T2_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "v1", baseType: ![[T1]]{{.*}})