This is an archive of the discontinued LLVM Phabricator instance.

[Legalizer][ARM][AArch64] Add legalizations for VECREDUCE_SEQ_FMUL
ClosedPublic

Authored by cameron.mcinally on Nov 2 2020, 1:45 PM.

Details

Summary

Hook up legalizations for VECREDUCE_SEQ_FMUL. This is following up on the VECREDUCE_SEQ_FADD work from D90247.

Diff Detail

Event Timeline

cameron.mcinally requested review of this revision.Nov 2 2020, 1:45 PM

Reformat to appease pre-merge checks...

nikic accepted this revision.Nov 3 2020, 1:07 PM

LGTM

Before you land this, I would suggest to improve the test coverage a bit:

  • In @llvm.vector.reduce.fmul.f128.v2f128, duplicate the tests without fast
  • In llvm/test/CodeGen/ARM/vecreduce-fmul-legalization-strict.ll and llvm/test/CodeGen/AArch64/vecreduce-fmul-legalization-strict.ll, use 1.0 instead of 0.0 as the start value. That was probably a copy&paste mistake from fadds.
This revision is now accepted and ready to land.Nov 3 2020, 1:07 PM
  • In llvm/test/CodeGen/ARM/vecreduce-fmul-legalization-strict.ll and llvm/test/CodeGen/AArch64/vecreduce-fmul-legalization-strict.ll, use 1.0 instead of 0.0 as the start value. That was probably a copy&paste mistake from fadds.

That caught my eye too, but the 0.0 seemed okay since we can't peep this without NSZ (-0*0) and NNAN (0*NaN). Changing it to 1.0 isn't a big deal though...

  • In llvm/test/CodeGen/ARM/vecreduce-fmul-legalization-strict.ll and llvm/test/CodeGen/AArch64/vecreduce-fmul-legalization-strict.ll, use 1.0 instead of 0.0 as the start value. That was probably a copy&paste mistake from fadds.

That caught my eye too, but the 0.0 seemed okay since we can't peep this without NSZ (-0*0) and NNAN (0*NaN). Changing it to 1.0 isn't a big deal though...

Oh, I see it now. The 1.0 is the neutral element, so it folds. Makes sense now...