This is an archive of the discontinued LLVM Phabricator instance.

[flang] Add lowering of move_alloc to IntrinsicCall
ClosedPublic

Authored by DavidTruby on Jan 12 2023, 8:05 AM.

Details

Summary

This patch relies on D141286 for the runtime implementation of
move_alloc.

Diff Detail

Event Timeline

DavidTruby created this revision.Jan 12 2023, 8:05 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added a subscriber: mehdi_amini. · View Herald Transcript
DavidTruby requested review of this revision.Jan 12 2023, 8:06 AM
clementval added inline comments.
flang/lib/Optimizer/Builder/Runtime/Allocatable.cpp
22–26

Please spell these 4 auto

flang/unittests/Optimizer/Builder/Runtime/AllocatableTest.cpp
18

Rebase and spelling correction

Spell out auto variables

Apart from the bot failure, this looks good to me.

flang/test/Lower/Intrinsics/move_alloc.f90
15

There is a build bot failure here because cast from fir.ref<fir.box> to fir.box are illegal:

error: loc("/var/lib/buildkite-agent/builds/llvm-project/flang/test/Lower/Intrinsics/move_alloc.f90":14:3): 'fir.convert' op invalid type conversion

But I expect it comes from the fact that this patch dependency is not set properly. You can add D141286 as a parent revision in the "Edit related revision" link in the revision menu (at the right of the patch description). (This is done automatically of you create you patch with a description that ends with "Depends on Dxxxx", but adding this line afterwards does nothing).

Rebase on top of runtime

jeanPerier added inline comments.Jan 19 2023, 1:25 AM
flang/unittests/Optimizer/CMakeLists.txt
34

Regarding the build bot failure, the cmake is likely missing:

DEPENDS
  FIRDialect
  FIRSupport
  HLFIRDialect
  ${dialect_libs}

which causes race condition when trying to compile the unit test files before the MLIR generated headers have been produced.

Add dependencies to CMake test file

Thanks for the suggested fix, I was wondering why this wasn't failing for me locally. Hopefully it will work this time!

The new failure doesn't look related to this patch, unless I'm mistaken?

jeanPerier accepted this revision.Jan 19 2023, 5:31 AM

LGTM, I do not think the windows fix has anything to do with your patch. Looks like a bot glitch when compiling the front-end.

This revision is now accepted and ready to land.Jan 19 2023, 5:31 AM

FYI, I have relaunched your patch build and it looks like both windows and linux succeeded, so that confirms the previous windows failure was unrelated.

This revision was automatically updated to reflect the committed changes.