This is an archive of the discontinued LLVM Phabricator instance.

[flang] Lower polymorphic entities types in dummy argument and function result
ClosedPublic

Authored by clementval on Sep 30 2022, 6:37 AM.

Details

Summary

This patch updates lowering to produce the correct fir.class types for
various polymorphic and unlimited polymoprhic entities cases. This is only the
lowering. Some TODOs have been added to the CodeGen part to avoid errors since
this part still need to be updated as well.
The fir.class<*> representation for unlimited polymorphic entities mentioned in
the document has been updated to fir.class<none> to avoid useless work in pretty
parse/printer.

This patch is part of the implementation of the poltymorphic
entities.
https://github.com/llvm/llvm-project/blob/main/flang/docs/PolymorphicEntities.md

Depends on D134957

Diff Detail

Event Timeline

clementval created this revision.Sep 30 2022, 6:37 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Sep 30 2022, 6:37 AM
clementval edited the summary of this revision. (Show Details)

A few questions. Overall looks great, I will have a more in depth look on Monday.

flang/include/flang/Optimizer/Dialect/FIROps.td
765

Will fir.embox -> fir.class will always be equivalent to fir.embox -> fir.box -> fir.convert -> fir.class, or will there be special cases where creating a fir.class via fir.embox would could not be implemented as a statically typed fir.box converted to a fir.class (I guess in order to be passed ? to a polymorphic dummy).

Doesn't Rebox_Op needs the same update ?

flang/include/flang/Optimizer/Dialect/FIRType.h
90

Can't fir::BaseBoxType replace the fir::BoxType and fir::ClassType occurrences ?

flang/lib/Lower/CallInterface.cpp
801

I dot not remember, is dynamicType.GetDerivedTypeSpec() not legal when dynamicType.IsUnlimitedPolymorphic() ?

clementval marked 2 inline comments as done.Oct 3 2022, 2:18 AM
clementval added inline comments.
flang/include/flang/Optimizer/Dialect/FIROps.td
765

As you mentioned some cases will require to update the type with the dynamic type.

Yes rebox will need the same update. I did not change it yet since there was no test case that cover it.

flang/include/flang/Optimizer/Dialect/FIRType.h
90

Yes. I'll update it.

flang/lib/Lower/CallInterface.cpp
801

dynamicType.GetDerivedTypeSpec() would return a nullptr when the dynamic type is unlimited polymorphic. So in that case we return directly the NoneType.

clementval updated this revision to Diff 464636.Oct 3 2022, 2:25 AM
clementval marked 2 inline comments as done.

Update isa_box_type

jeanPerier accepted this revision.Oct 3 2022, 9:15 AM

Other than the TYPE(*) test, LGTM.

flang/test/Lower/polymorphic-types.f90
157

Would it make sense to add a TYPE(*) test, or is it not yet possible to lower them ?

This revision is now accepted and ready to land.Oct 3 2022, 9:15 AM
clementval marked 2 inline comments as done.Oct 3 2022, 9:43 AM
clementval added inline comments.
flang/test/Lower/polymorphic-types.f90
157

TYPE(*) is in fact already lowered. I'll add tests before pushing. I'm planning a separate patch to update the doc and add an attribute for TYPE(*) representation.

This revision was landed with ongoing or failed builds.Oct 4 2022, 12:44 AM
This revision was automatically updated to reflect the committed changes.
clementval marked an inline comment as done.