Page MenuHomePhabricator

[ARM] Added additional patterns to VABD instruction
ClosedPublic

Authored by MeeraN on Jul 24 2020, 1:57 AM.

Details

Summary

Added additional patterns to VABD instruction so that it will be selected in place of VSUB then VABS.

Diff Detail

Event Timeline

MeeraN created this revision.Jul 24 2020, 1:57 AM
MeeraN created this object with visibility "All Users".
MeeraN created this object with edit policy "No One".

Hi Meera, two comments:

  • you're adding patterns for f16 and f32 vectors, so would be good to cover both cases and thus have tests for vabd.f16 and vabd.f32.
  • since we're just matchings a fabs + fsub patterrn, can you reduce the test case?
MeeraN updated this revision to Diff 280402.Jul 24 2020, 4:25 AM

Added second test for f16 case and made test cases much shorter. Also moved position of pattern definition so that the size is handled clearly

MeeraN changed the visibility from "All Users" to "Public (No Login Required)".Jul 24 2020, 6:21 AM
MeeraN changed the edit policy from "No One" to "All Users".
SjoerdMeijer requested changes to this revision.Jul 24 2020, 6:22 AM

Cheers, this is exactly what I was expecting, one nit inlined.

llvm/test/CodeGen/Thumb2/mve-vabd.ll
3

For bonus points: probably best to add an extra RUN line with just -mattr=+mve so that we test this case too, i.e. HasMVEFloat isn't set.

This revision now requires changes to proceed.Jul 24 2020, 6:22 AM
SjoerdMeijer added inline comments.Jul 24 2020, 6:43 AM
llvm/test/CodeGen/Thumb2/mve-vabd.ll
26

one more question: is it strictly necessary to add the fast attribute?

MeeraN updated this revision to Diff 280461.Jul 24 2020, 7:31 AM

Removed fast attributes and added test cases for when the floating point flag is not set.

SjoerdMeijer accepted this revision.Jul 24 2020, 8:35 AM

Cheers, LGTM.

This revision is now accepted and ready to land.Jul 24 2020, 8:35 AM
This revision was automatically updated to reflect the committed changes.
MeeraN marked 2 inline comments as done.