This is an archive of the discontinued LLVM Phabricator instance.

[flang] Support allocate with source for polymorphic entities
ClosedPublic

Authored by clementval on Jan 16 2023, 12:51 AM.

Details

Summary

Apply the source type spec to the descriptor for
polyrmophic entities.

Diff Detail

Event Timeline

clementval created this revision.Jan 16 2023, 12:51 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 16 2023, 12:51 AM
clementval requested review of this revision.Jan 16 2023, 12:51 AM

Use alloc object rank instead of source rank

Questions about some edge cases, the rest looks good.

flang/lib/Lower/Allocatable.cpp
578

Is it possible for the sourceExpr->GetType() to be null ?

Is it also possible that the source be an unlimited polymorphic if the allocatable object also is one ?

subroutine foo(any, any_source)
 class(*), allocatable :: any
 class(*), pointer :: any_source
 allocate(any, source = any_source)
end subroutine

(If so, this will probably require some custom runtime entry point, and it's fine to leave a TODO at first).

Simplify condition + add test for unlimited polymorphic allocation from
unlimited polyrmophic source.

clementval marked an inline comment as done.Jan 17 2023, 1:22 AM
clementval added inline comments.
flang/lib/Lower/Allocatable.cpp
578

I believe sourceExpr->GetType() is always defined.

It is possible to have an unlimited polymorphic source to allocate an unlimited polymorphic source. This falls in the TypeCategory::Derived category as unlimited polymorphic descriptor are defined as DerivedType in the frontend.

jeanPerier added inline comments.Jan 17 2023, 2:35 AM
flang/lib/Lower/Allocatable.cpp
578

But what if the unlimited polymorphic dynamic type is an intrinsic type and not a derived type (say any_source is pointing to an integer). Is it correct to call genInitDerived in that case ?

578

I believe sourceExpr->GetType() is always defined.

Ok, makes sense. I was not sure. However, I remember that in some rare semantics bugs, it happened that GetType() returned null (because something failed in expression resolution and was not correctly reported), so I would tend add asserts when using that (or using the DEREF() front end macro).

clementval marked 2 inline comments as done.Jan 17 2023, 5:55 AM
clementval added inline comments.
flang/lib/Lower/Allocatable.cpp
578

Yeah I was thinking about this after the update. This would need to go through a custom runtime entry point. I'll make a separate patch for this.

578

An assert makes sense.

clementval marked an inline comment as done.
  • Add a TODO for unlimited polymoprhic source and allocate object
  • Add an assert for GetType()
PeteSteinfeld accepted this revision.Jan 17 2023, 6:50 AM

Other than some clang-format diffs, all builds and tests correctly and looks good.

flang/lib/Lower/Allocatable.cpp
570–575

I get clang-format diffs here.

627

For some reason this comment generates a clang-format diff.

634–638

More clang-format diffs here.

646–650

More clang-format diffs here.

676–680

More clang-format diffs.

This revision is now accepted and ready to land.Jan 17 2023, 6:50 AM
clementval marked 6 inline comments as done.Jan 17 2023, 7:10 AM
clementval added inline comments.
flang/lib/Lower/Allocatable.cpp
570–575

I guess it was before the last update. Pre-commit check is clean right now.

clementval marked an inline comment as done.

Rebase after MOLD patch

This revision was landed with ongoing or failed builds.Jan 17 2023, 7:12 AM
This revision was automatically updated to reflect the committed changes.