[all-commits] [llvm/llvm-project] af9137: [flang][debug] Improve handling of cyclic derived ...

[Abid Qadeer via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: af91372b75613d5654e68d393477e8621cb93da7
      https://github.com/llvm/llvm-project/commit/af91372b75613d5654e68d393477e8621cb93da7
  Author: Abid Qadeer <haqadeer at amd.com>
  Date:   2025-01-20 (Mon, 20 Jan 2025)

  Changed paths:
    M flang/lib/Optimizer/Dialect/FIRType.cpp
    M flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp
    A flang/test/Integration/debug-cyclic-derived-type-3.f90

  Log Message:
  -----------
  [flang][debug] Improve handling of cyclic derived types. (#122770)

When `RecordType` is converted to corresponding `DIType`, we cache the
information to avoid doing the conversion again.

Our conversion of `RecordType` looks like this:

`ConvertRecordType(RecordType Ty)`
1. If type `Ty` is already in the cache, then return the corresponding
item.
2. Create a place holder `DICompositeTypeAttr` (called `ty_self` below)
for `Ty`
3. Put `Ty->ty_self` in the cache
4. Convert members of `Ty`. This may cause `ConvertRecordType` to be
called again with other types.
5. Create final `DICompositeTypeAttr`
6. Replace the `ty_self` in the cache with one created in step 5 end


The purpose of creating `ty_self` is to handle cases where a member may
have reference to parent type.

Now consider the code below:

```
type t1
  type(t2), pointer :: p1
end type
type t2
   type(t1), pointer :: p2
end type
```

While processing t1, we could have a structure like below. `t1 -> t2 ->
t1_self`

The `t2` created during handling of `t1` cant be cached on its own as it
contains a place holder reference. It will fail an assert in MLIR if it
is processed standalone. To avoid this problem, we have a check in the
step 6 above to not cache such types. But this check was not tight
enough. It just checked if a type should not have a place holder
reference to another type. It missed the following case where the place
holder reference can be in a type further down the line.

```
type t1
  type(t2), pointer :: p1
end type
type t2
  type(t3), pointer :: p2
end type
type t3
  type(t1), pointer :: p3
end type
```

So while processing `t1`, we have to stop caching of not only `t3` but
also of `t2`. This PR improves the check and moves the logic inside
`convertRecordType`.

Please note that this limitation of why a type cant have a placeholder
reference is because of how such references are resolved in the mlir.
Please see the discussion at the end of this
[PR](https://github.com/llvm/llvm-project/pull/106571).

I have to change `getDerivedType` so that it will also get the derived
type for things like `type(t2), pointer :: p1` which are wrapped in
`BoxType`. Happy to move it to a new function or a local helper in case
this change is problematic.

Fixes #122024.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications