This is an archive of the discontinued LLVM Phabricator instance.

[flang] Use Assign() runtime for copy-in/copy-out.
ClosedPublic

Authored by vzakhari on Dec 20 2022, 5:42 PM.

Details

Summary

The loops generated under IsContiguous check for copy-in/copy-out
result in LLVM backend spending too much time optimizing them.
At the same time, the copy loops do not provide any optimization
opportunities with the surrounding code (since they are executed
under runtime IsContiguous check), so the copy code may be optimized
on its own and this can be done in runtime.

I thought I could implement and use new APIs for packing/unpacking
non-contiguous data (interfaces added in D136378), but then I found
that Assign() is already doing what is needed. If performance
becomes an issue for these loops, we can optimize code in Assign()
rather than creating new APIs.

Thus, this change makes use of Assign() for copy-in/copy-out
of boxed objects, and this is done only if the objects
are non-contiguous during execution. Copies for non-boxed
objects (e.g. for passing as VALUE dummy argument) are still
done inline, because they can potentially be optimized with
surrounding loops.

I added internal -inline-copyinout-for-boxes option to revert to the old
behavior just to make it easier to triage performance regressions,
if any appear after the change.

CPU2017/521.wrf compiles for 2179 seconds without the change and
the module_dm.f90 compiled with -O0 (at -O2 this single
module compiles for 5775 seconds). With the change total compilation
time of the benchmark reduces to 722 seconds.

Diff Detail

Event Timeline

vzakhari created this revision.Dec 20 2022, 5:42 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: mehdi_amini. · View Herald Transcript
vzakhari requested review of this revision.Dec 20 2022, 5:42 PM

All builds and tests correctly and looks good. Someone who understands this code better than I do should approve, though.

flang/lib/Lower/ConvertExpr.cpp
110

Should read "call the Fortran"

jeanPerier accepted this revision.Dec 21 2022, 12:34 AM

Looks good to me

flang/lib/Lower/ConvertExpr.cpp
2295

nit: braces not needed.

This revision is now accepted and ready to land.Dec 21 2022, 12:34 AM
vzakhari added inline comments.Dec 21 2022, 8:50 AM
flang/lib/Lower/ConvertExpr.cpp
110

Thank you, Pete. Will fix it.

2295

Thank you for the review, Jean!

I tried to follow https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md#layout :)
Steve recently mentioned that it would be good to gradually adjust Lower and Optimizer to use Flang coding style.

vzakhari edited the summary of this revision. (Show Details)Dec 21 2022, 9:25 AM

Fixed miss-copy-paste in the description: 246 => 722.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 21 2022, 9:55 AM
tblah added a subscriber: tblah.Dec 22 2022, 2:24 AM