This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Add scalable vectorisation tests for int/FP <> int/FP conversions
ClosedPublic

Authored by david-arm on Apr 6 2021, 6:30 AM.

Details

Summary

We can already vectorise loops that involve int<>int, fp<>fp, int<>fp
and fp<>int conversions, however we didn't previously have any tests
for them. This patch adds some tests for each conversion type.

Diff Detail

Event Timeline

david-arm requested review of this revision.Apr 6 2021, 6:30 AM
david-arm created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 6:30 AM
fhahn added a comment.Apr 9 2021, 6:04 AM

Would it make sense to have all the conversion tests in a single file, as the tests are quite compact?

llvm/test/Transforms/LoopVectorize/AArch64/sve-conv-int-to-int.ll
6 ↗(On Diff #335497)

Do we still need this? IIRC most those checks have been removed by now?

Sure, I can combine them into a single test if you think that's better? I wasn't sure whether in general we prefer to have more concise test files or to have fewer, larger files. I could create a file with a name like sve-type-conv.ll.

llvm/test/Transforms/LoopVectorize/AArch64/sve-conv-int-to-int.ll
6 ↗(On Diff #335497)

Yes you're right. When I created the patch originally I think we still needed them, but I'll rebase and remove them.

fhahn added a comment.Apr 9 2021, 6:15 AM

Sure, I can combine them into a single test if you think that's better? I wasn't sure whether in general we prefer to have more concise test files or to have fewer, larger files. I could create a file with a name like sve-type-conv.ll.

Sounds good to me. I don't think there's a general answer really. In this case it seems like they all test the same/similar code path and there might be less processing overhead with fewer, larger files than with many small files.

david-arm updated this revision to Diff 336453.Apr 9 2021, 7:43 AM
  • Removed warnings check line.
  • Combined all tests into one file.
david-arm marked an inline comment as done.Apr 9 2021, 7:43 AM
kmclaughlin added inline comments.Apr 23 2021, 3:39 AM
llvm/test/Transforms/LoopVectorize/AArch64/sve-type-conv.ll
156–157

I'm not sure why this test and the one below are using both a [u|s]itofp and an fptrunc, is it possible to just use uitofp i64 %0 to half here?

217

nit: Should this test be called @u8_to_u16?

david-arm updated this revision to Diff 339971.Apr 23 2021, 3:57 AM
  • Removed unnecessary fptrunc from tests
david-arm marked 2 inline comments as done.Apr 23 2021, 4:00 AM
david-arm added inline comments.
llvm/test/Transforms/LoopVectorize/AArch64/sve-type-conv.ll
156–157

Yeah, this is an artefact of clang. When compiling a loop such as this:

void foo(float16_t *dst, uint64_t *src, long long n) {
  for (long long i = 0; i < n; i++)
    dst[i] = src[i];
}

it generates IR with two-part conversions, i.e. uitfop + fptrunc. Not sure why, but I've changed the test anyway.

kmclaughlin accepted this revision.Apr 23 2021, 5:09 AM

Thanks for updating the tests, LGTM!

This revision is now accepted and ready to land.Apr 23 2021, 5:09 AM
This revision was automatically updated to reflect the committed changes.
david-arm marked an inline comment as done.