This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Test more types in sve-fixed-length-subvector.ll
ClosedPublic

Authored by joechrisellis on Mar 16 2021, 3:03 AM.

Details

Summary

Previously only the i32 type was tested. Now, the {i,f}{16,32,64} types
are tested.

The v8{i,f}16 cases lower differently to the other cases, which is worth
defending. The lowering for the other cases is currently identical, but
probably worth having for the better coverage.

Diff Detail

Event Timeline

joechrisellis created this revision.Mar 16 2021, 3:03 AM
joechrisellis requested review of this revision.Mar 16 2021, 3:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2021, 3:03 AM

I think this needs a closer look at the CHECK labels - I see 'CHECK_GE' used where I think 'VBITS_GE' is intended. Also, 'CHECKT', which I take it is not intended.

I'm also not sure about introducing a test beyond the fundamental limitation of 2048 bits. I think it should be dropped for now, at a minimum it would be inconsistent with other files.

Remove the -aarch64-sve-vector-bits-min=4096 case; the architectural maximum is 2048 bits, so this case is erroneous.

Amend labels.

david-arm added inline comments.Mar 17 2021, 2:47 AM
llvm/test/CodeGen/AArch64/sve-fixed-length-subvector.ll
51

Should this just be a normal CHECK line consistent with the v8i32 case (same size as v16i16)?

65

VBITS_GE_512 consistent with the v16i32 case?

79

VBITS_GE_1024?

203

Same comments as the v16i16 and other cases.

joechrisellis updated this revision to Diff 331230.EditedMar 17 2021, 4:53 AM
joechrisellis marked 4 inline comments as done.

Address review comments.

  • @david-arm: thanks! I had forgotten to update the check line to reflect the different vector size. Should be correct now.
This revision is now accepted and ready to land.Mar 18 2021, 3:52 AM