Page MenuHomePhabricator

[AArch64] Generate vector signed/unsigned mul and mla/mls long

Authored by bmakam on Oct 2 2014, 2:38 PM.



Dear Tim, Chad, Ana, Jiangning and Hao,

This is my first patch to the LLVM project.

We are seeing missed opportunities to generate widened mul/mla/mls in AArch64
ARM had an implementation that was ported to AArch64.

Diff Detail

Event Timeline

bmakam updated this revision to Diff 14348.Oct 2 2014, 2:38 PM
bmakam retitled this revision from to [AArch64] Generate vector signed/unsigned mul and mla/mls long.
bmakam updated this object.
bmakam edited the test plan for this revision. (Show Details)
bmakam added a subscriber: Unknown Object (MLST).
t.p.northover edited edge metadata.Oct 2 2014, 3:07 PM

Hi Balaram,

Thanks for working on this. I think the code is mostly OK (which is a relief, otherwise ARM would already be broken). I've got a few small suggestions though:


Function names should start with a lower case letter (this isn't the only example).


It's not called VMULL on AArch64.




Could you comment on which CPUs this is known to be beneficial for.


CHECK-LABEL. It's also a good idea to include the label's ':' in the check line, otherwise you can match weird directives at the end of the function like

.size	umull_extvec_v2i32_v2i64, .Ltmp1-umull_extvec_v2i32_v2i64
bmakam updated this revision to Diff 14379.Oct 3 2014, 7:20 AM
bmakam edited edge metadata.

updated patch addressing Tim's comments.

mcrosier added inline comments.Oct 3 2014, 7:26 AM

Please be consistent with your CHECK directives. Specifically, either do or don't include a space between the ; and the CHECK for each test. Personally, I prefer a space and I believe this is the general style within the community.

bmakam updated this revision to Diff 14382.Oct 3 2014, 8:49 AM

Updated patch after addressing both Chad's and Tim's comments.

bmakam updated this revision to Diff 14391.Oct 3 2014, 11:43 AM

Actually, including the label's ':' in the check line in this commit. Previously, I only changed CHECK with CHECK-LABEL


bmakam updated this revision to Diff 14411.Oct 3 2014, 3:07 PM
t.p.northover accepted this revision.Oct 6 2014, 9:44 AM
t.p.northover edited edge metadata.

This looks OK to me now. Thanks for updating it.


This revision is now accepted and ready to land.Oct 6 2014, 9:44 AM
mcrosier edited edge metadata.Oct 7 2014, 7:49 PM

Committed in r219276.

Would you like for me to add some logic so this is only enabled for A53/A57 or are you find with this being enabled for Cyclone as well?


mcrosier closed this revision.Oct 7 2014, 7:50 PM
bmakam added a comment.Oct 7 2014, 7:58 PM

Thanks Tim and Chad :)

Congrats and welcome to the club, Balaram.