This is an archive of the discontinued LLVM Phabricator instance.

[ARM][MVE] VCTP instruction selection
ClosedPublic

Authored by samparker on Sep 9 2019, 2:54 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Sep 9 2019, 2:54 AM

I'm also wondering how this fits in with D67085...

I'm also wondering how this fits in with D67085...

Should be fine so long as they are not loading or storing anything (or can get it to not load or store anything!)

lib/Target/ARM/ARMInstrMVE.td
3946 ↗(On Diff #219307)

Nitpicking: Indent!

test/CodeGen/Thumb2/mve-vctp.ll
1 ↗(On Diff #219307)

Might as well add -verify-machineinstrs. Should the triple be in full (-none-eabi)? I'm not sure it matters a lot, but may default to linux otherwise.

Also I would just run the update script on a file this size.

24 ↗(On Diff #219307)

It's probably best to use the <4 x i1> as opposed to returning it (in a vselect or something).

I'm not sure if the calling convention for a v4i1 is super very well defined. It's not defined from C, so only comes up in llvm test and if we can just sidestep that problem...

samparker updated this revision to Diff 219320.Sep 9 2019, 4:14 AM

Thanks. Addressed comments.

dmgreen accepted this revision.Sep 9 2019, 4:41 AM

LGTM

test/CodeGen/Thumb2/mve-vctp.ll
14 ↗(On Diff #219320)

If you add arm_aapcs_vfpcc, you can pass the <16 x i8> vectors sanely are args/return values, if you want to remove the extra vldrs and vstrs.

This revision is now accepted and ready to land.Sep 9 2019, 4:41 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2019, 5:55 AM