This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Adding Neon Polynomial vadd Intrinsics
ClosedPublic

Authored by rsanthir.quic on Feb 16 2021, 5:43 PM.

Details

Summary

This patch adds the following intrinsics:

vadd_p8
vadd_p16
vadd_p64
vaddq_p8
vaddq_p16
vaddq_p64
vaddq_p128

Diff Detail

Event Timeline

rsanthir.quic created this revision.Feb 16 2021, 5:43 PM
rsanthir.quic requested review of this revision.Feb 16 2021, 5:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2021, 5:43 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This is the final of three patches to address the following:
https://bugs.llvm.org/show_bug.cgi?id=47828

rebased due to merge issues

t.p.northover accepted this revision.Feb 17 2021, 2:00 AM

Looks sensible to me.

This revision is now accepted and ready to land.Feb 17 2021, 2:00 AM

This LGTM unless @labrinea has any final comments.

Could you add "Neon" in the commit title? "[AArch64] Adding Neon Polynomial vadd Intrinsics support", just to help when reading git history.

clang/test/CodeGen/aarch64-poly-add.c
5

Do we need this option? (-ffp-contract)

rsanthir.quic marked an inline comment as done.Feb 17 2021, 9:23 AM
rsanthir.quic added inline comments.
clang/test/CodeGen/aarch64-poly-add.c
5

You are correct this is not necessary, I'll remove it

rsanthir.quic marked an inline comment as done.
rsanthir.quic retitled this revision from [AArch64] Adding Polynomial vadd Intrinsics support to [AArch64] Adding Neon Polynomial vadd Intrinsics.

removed unnecessary flag in test and changed commit message.

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Feb 18 2021, 12:09 PM

Looks like this breaks check-clang: http://45.33.8.238/win/33493/step_7.txt

Please take a look, and revert for now if it takes a while to fix.

Thanks for the revert! FWIW the failure repros for me locally if I just do ninja check-clang (as long as I have the aarch64 target enabled, which it is by default in the cmake build).

rsanthir.quic reopened this revision.Feb 19 2021, 10:07 AM
This revision is now accepted and ready to land.Feb 19 2021, 10:07 AM

Windows builds were failing due to missing builtins in Intrinsics map

rebased on main

ctetreau accepted this revision.Feb 19 2021, 2:45 PM
ctetreau added a subscriber: ctetreau.

LGTM. I'll land it for you.

This revision was automatically updated to reflect the committed changes.