This is an archive of the discontinued LLVM Phabricator instance.

[Flang] Fix ALLOCATE with MOLD where MOLD is a scalar
ClosedPublic

Authored by PeteSteinfeld on Mar 6 2023, 1:24 PM.

Details

Summary

We were failing tests where an ALLOCATE statement that allocated an
array had a non-character scalar MOLD argument.

I fixed this be merging the code for ALLOCATE statements with MOLD
and SOURCE arguments.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.Mar 6 2023, 1:24 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 6 2023, 1:24 PM
PeteSteinfeld requested review of this revision.Mar 6 2023, 1:24 PM
PeteSteinfeld edited the summary of this revision. (Show Details)Mar 6 2023, 1:25 PM
jeanPerier added inline comments.Mar 7 2023, 12:33 AM
flang/lib/Lower/Allocatable.cpp
223

I do not think you should reverse this. Even if unused by this test, this is still broken code. You could probably still expose the issue with a mold allocation to a character scalar allocatable.

376

What you did is correct for simple numerical objects, but the character case will still be buggy I think (I did not check, but I suspect the bounds will not be set properly for allocate(some_char_array(10,20), mold=scalar_char)).
I think it is also a bit risky to completely ignore the mold here, there are some other cases where the MOLD may be relevant: at least polymorphism, parametrized derived types.

If what is missing are the bounds-spec application, it would make more sense to me to set the bounds properly in genMoldAllocation.

Pushing this a bit further, the code for SOURCE and MOLD allocation should be exactly similar except the last runtime call (genRuntimeAllocate for MOLD vs genRuntimeAllocateSource for SOURCE). Source allocation is a MOLD allocation followed by an assignment (RuntimeAllocateSource simply calls RuntimeAllocate and some assign version).

Could you try sharing genSourceAllocation above for both source and MOLD allocation except for last runtime call?

clementval added inline comments.Mar 7 2023, 12:34 AM
flang/lib/Lower/Allocatable.cpp
373–374

This will not work with polymorphic entities and unlimited polymorphic entities where we need to get the dynamic type information from the MOLD.

class(*), allocatable :: a

allocate(a, mold=3)

Changes after excellent feedback from Jean and Valentin.

PeteSteinfeld edited the summary of this revision. (Show Details)Mar 8 2023, 12:39 PM
clementval accepted this revision.Mar 9 2023, 1:33 AM

LGTM

flang/lib/Lower/Allocatable.cpp
373–374

You can also remove the brace here to be consistent

375

brace

This revision is now accepted and ready to land.Mar 9 2023, 1:33 AM
This revision was automatically updated to reflect the committed changes.
PeteSteinfeld marked 2 inline comments as done.