This is an archive of the discontinued LLVM Phabricator instance.

[flang] Add tests for converting arrays and refs to arrays. NFC
ClosedPublic

Authored by rovka on Jan 10 2022, 3:53 AM.

Details

Summary

Cover some of the code paths from LLVMTypeConverter::convertPointerLike
and LLVMTypeConverter::convertSequenceType (specifically related to
whether or not the sequence type has a constant interior).

Diff Detail

Event Timeline

rovka created this revision.Jan 10 2022, 3:53 AM
rovka requested review of this revision.Jan 10 2022, 3:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2022, 3:53 AM

Thanks for adding these! I think that I've identified a couple more cases that are not covered here. Would it make sense to test them as well?

The conversions are not 100% clear to me TBH. For example, !fir.ref<!fir.array<10x?x11xf32>> becomes a plain !llvm.ptr (with no array inside) because the number of rows is unknown, correct?

flang/test/Fir/types-to-llvm.fir
6–7
39
55

There's a switch from column-major to row-major here, right? (hence 11 and 10 are swapped). Might be worth adding a comment.

rovka updated this revision to Diff 398620.Jan 10 2022, 7:05 AM
  • Cover more code paths (dynamic and fixed-length chars, ref<box>).
  • Add a comment that we're switching between column-major and row-major, and also modify the first test in the file to show this (was 10x10, is now 10x12 so we can see the difference).

Thanks for the comments, let me know if I missed any other cases :)

awarzynski accepted this revision.Jan 10 2022, 9:19 AM

I think that this covers all cases :)

This revision is now accepted and ready to land.Jan 10 2022, 9:19 AM