This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fix associating entity when selector is an array, pointer or allocatable
ClosedPublic

Authored by clementval on Dec 14 2022, 5:53 AM.

Details

Summary

In SELECT TYPE, within the block following TYPE IS, the associating entity is not polymorphic.
It has the type named in the type guard and other properties taken from the
selector. Within the block following a CLASS IS type guard statement, the
associating entity is polymorphic and has the declared type named in the type
guard statement.
This patch makes sure the associating entity matches the selector if it is
an array, a pointer or an allocatable.

Diff Detail

Event Timeline

clementval created this revision.Dec 14 2022, 5:53 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 14 2022, 5:53 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Dec 14 2022, 5:53 AM
clementval edited the summary of this revision. (Show Details)Dec 14 2022, 5:56 AM

Fix ref type for box addr

jeanPerier added inline comments.Dec 14 2022, 7:04 AM
flang/lib/Lower/Bridge.cpp
2317

I think it may be possible to simplify this a lot by using genType(symbol) where symbol is the symbol you are fetching in addAssocEntitySymbol. It should directly come with the right type and attributes. Then, you should not have to deal with all the type building manually and can focus on deciding if the value can be unboxed or not.

Assuming the input Exv is a fir::BoxValue, you could use BoxValue.clone(casted box) to build a new boxValue (or fir::substbase(newbase, exv) to also deal with pointers/allocatables).

Then, for non polymorphic, non pointer/allocatable/ non pdt, scalars and contiguous arrays, you could use fir::factory::readBoxValue to get the new ExtendedValue that must be mapped.

2351

I am not sure it is correct to use a raw address and not a box unless the array is known to be contiguous.

clementval added inline comments.Dec 14 2022, 7:57 AM
flang/lib/Lower/Bridge.cpp
2317

I think it may be possible to simplify this a lot by using genType(symbol) where symbol is the symbol you are fetching in addAssocEntitySymbol. It should directly come with the right type and attributes.

The symbol just carries the correct element type but nothing else.

I'll update the rest.

@jeanPerier should approve before you merge, but I can confirm that all builds, tests, and is formatted correctly.

  • Cleanup a bit the code where possible
  • Keep array emboxed since we do not have the information if they are contiguous or not.
  • use fur::BoxValue.clone where possible
clementval added inline comments.Dec 14 2022, 10:04 AM
flang/lib/Lower/Bridge.cpp
2317

fir::factory::readBoxValue does not work with unlimited polymorphic descriptor so I simplified the code where it was possible.

2351

I changed to keep the array emboxed.

jeanPerier accepted this revision.Dec 15 2022, 2:09 AM

LGTM

flang/lib/Lower/Bridge.cpp
2317

The symbol just carries the correct element type but nothing else.

Thanks for trying. I'll try to fix genType at some point. It is most likely wrong that it cannot deal with symbols with AssocEntityDetails properly. I am ok with what you have here.

This revision is now accepted and ready to land.Dec 15 2022, 2:09 AM