This is an archive of the discontinued LLVM Phabricator instance.

[sve][acle] Add reinterpret intrinsics for brain float.
ClosedPublic

Authored by fpetrogalli on Jun 24 2020, 2:17 PM.

Diff Detail

Event Timeline

fpetrogalli created this revision.Jun 24 2020, 2:17 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 24 2020, 2:17 PM
david-arm added inline comments.Jun 24 2020, 11:55 PM
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_reinterpret-bfloat.c
5

Hi @fpetrogalli, in the same way that you asked @kmclaughlin if she could add the ASM-NOT check line in her patch, are you able to do that here? You'd need to add an additional RUN line though to compile to assembly. Don't worry if it's not possible though!

fpetrogalli marked 2 inline comments as done.Jun 25 2020, 8:00 AM
fpetrogalli added inline comments.
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_reinterpret-bfloat.c
5

It is possible, but my understanding is that we anyway decided to do this work as a separate patch.

fpetrogalli marked an inline comment as done.

@david-arm, at the end I decided to add the ASM-NOT test, it was easy and came for free.

Also, I have moved the IR tests in the file with all other bitcasts, using a funcion attribute to enable the bf16 feature only for those functions that deal with bfloats.

david-arm accepted this revision.Jun 26 2020, 12:06 AM

Can you remove the duplicate tests before submitting? Otherwise LGTM!

llvm/test/CodeGen/AArch64/sve-bitcast-bfloat.ll
9

Aren't these tests all duplicates of ones in llvm/test/CodeGen/AArch64/sve-bitcast.ll? Looks like you can remove this file completely.

This revision is now accepted and ready to land.Jun 26 2020, 12:06 AM
fpetrogalli marked 2 inline comments as done.Jun 26 2020, 7:23 AM
fpetrogalli added inline comments.
llvm/test/CodeGen/AArch64/sve-bitcast-bfloat.ll
9

(facepalm). yes, I remember thinking "I have to remove them before updating the patch", and then I forgot... I will do it before submitting. Thank you for pointing this out.

fpetrogalli marked an inline comment as done.

I removed the duplicate tests.

This revision was automatically updated to reflect the committed changes.

Ops, I accidentally removed the C tests... I'll revert, add the tests and recommit.