[flang][debug] Unexpected Assertion `!attr.getIsRecSelf() && "unbound DI recursive self reference"' failure #122024
Comments
|
@abidh, could you please take a look? |
EugeneZelenko
added
debuginfo
flang:ir
crash
|
@llvm/issue-subscribers-flang-ir Author: JL (jieljiel)
flang crashes with -g on the following example.
|
|
@llvm/issue-subscribers-debuginfo Author: JL (jieljiel)
flang crashes with -g on the following example.
|
|
I was about to file a new bug report, but fortunately, I've found this one. I'm facing the same assertion failing when building dbcsr (one of the CP2K dependencies) with |
|
As a side note, building CP2K's dbcsr seems to be broken again due to some recent commit, it fails like that: |
abidh
added a commit
to abidh/llvm-project
that referenced
this issue
Jan 13, 2025
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 agin 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. We previously had a check in the step 6 above to not cache it. 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 the following PR. llvm#106571 Fixes llvm#122024.
|
Hi @jieljiel, @pawosm-arm @pawosm-arm |
Hi @abidh, the most painful moment when building CP2K is the preparation of all of the required dependencies (namely, sufficient version of OpenMPI, e.g. 5.0.6, fftw with OpenMP support, OpenBLAS from the To trigger the build: The dbcsr will be built first automatically, remember to clone CP2K recursively, so the dbcsr repo will be cloned too. |
|
Hi @pawosm-arm The PR is approved. If you could confirm that it fixes the issue that you observed then I will merge it. |
|
Hi @abidh doing anything with CP2K takes several hours (days if you want to build all of it), I'm starting it now to build up until this know failure point, but you'd have to wait for it if you want to be sure. |
|
@abidh I've managed to prepare building environment for CP2K on a brand new host, and I can confirm that with the patch from your PR applied, I can build |
abidh
added a commit
that referenced
this issue
Jan 20, 2025
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](#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
flang crashes with -g on the following example.
The text was updated successfully, but these errors were encountered: