Page MenuHomePhabricator

[NEON] Support VLD1xN intrinsics in AArch32 mode (Clang part)
ClosedPublic

Authored by kosarev on May 20 2018, 11:34 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

kosarev created this revision.May 20 2018, 11:34 AM

Had only a first brief look; see some first drive by comments inline.

lib/CodeGen/CGBuiltin.cpp
7865 ↗(On Diff #147718)

How about this FIXME? Is it still relevant? And does it need to be moved up? Or perhaps better: move the code back here to minimise the diff?

test/CodeGen/arm-neon-vld.c
4 ↗(On Diff #147718)

Should this be armv7?

Thanks for reviewing.

lib/CodeGen/CGBuiltin.cpp
7865 ↗(On Diff #147718)

Yes, it's still true for the vst builtins handled below. None of the vld/vst patches removes this comment, but it should go away with whatever is the one to be committed last.

Umm, it seems leaving the vld code here wouldn't make the diff smaller?

test/CodeGen/arm-neon-vld.c
4 ↗(On Diff #147718)

There are more ARMv8 vld intrinsics that we currently support only in A64 so I was going to add tests for them here. I'm not sure if we want to test availability of NEON intrinsics for various architectures with codegen tests like this one or have some separate tests in sema.

SjoerdMeijer accepted this revision.May 31 2018, 3:48 AM

I agree: these intrinsics are available in v7/A32/A64.

lib/CodeGen/CGBuiltin.cpp
7865 ↗(On Diff #147718)

ah yes, nevermind, got confused here.

This revision is now accepted and ready to land.May 31 2018, 3:48 AM
This revision was automatically updated to reflect the committed changes.