This is an archive of the discontinued LLVM Phabricator instance.

[flang] Implement the runtime portion of the UNPACK intrinsic
ClosedPublic

Authored by PeteSteinfeld on Jul 19 2021, 11:43 AM.

Details

Summary

I'd previously merged this into the fir-dev branch. This change is to
do the same thing to the main branch of llvm-project.

Diff Detail

Event Timeline

PeteSteinfeld requested review of this revision.Jul 19 2021, 11:43 AM
PeteSteinfeld created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2021, 11:43 AM
PeteSteinfeld added a project: Restricted Project.
mleair added inline comments.Jul 19 2021, 12:12 PM
flang/runtime/transformational.cpp
524 ↗(On Diff #359859)

Same comment as before. llvm style does not like braces for single code blocks.

PeteSteinfeld added inline comments.Jul 19 2021, 12:14 PM
flang/runtime/transformational.cpp
524 ↗(On Diff #359859)

And as I mentioned in the other pull request, I don't think we're following this convention for flang code.

mleair added inline comments.Jul 19 2021, 12:14 PM
flang/runtime/transformational.cpp
524 ↗(On Diff #359859)

Saw you comment in your cshift review. So, disregard.

jeanPerier added inline comments.Jul 20 2021, 1:14 AM
flang/runtime/transformational.cpp
524 ↗(On Diff #359859)

It looks ok to me to have it the way you changed it, but I am surprised you had to change it. Isn't field.IncrementSubscripts(fieldAt) a no-op on scalar field ? Did you run into bugs without your change ?

PeteSteinfeld added inline comments.Jul 20 2021, 9:49 AM
flang/runtime/transformational.cpp
524 ↗(On Diff #359859)

This is weird. I thought I had a test that failed without this change, but I just tried to reproduce it, and I can't. So I'll revert the code.

Responding to review comments.

This revision is now accepted and ready to land.Jul 21 2021, 4:07 AM