[PATCH] D89162: [SVE] Lower fixed length VECREDUCE_SEQ_FADD operation

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added a comment.

In D89162#2347701 <https://reviews.llvm.org/D89162#2347701>, @paulwalker-arm wrote:

> Sorry @cameron.mcinally I've not had much time for code reviews this week although will take proper look tomorrow.  I have a question though.  You've added extra legalisation support but I don't see any explicit tests (or at least ones with matching check lines) for it.  Is this something you need for this patch? (I'm guessing sve-fixed-length-fp-reduce.ll's stock NEON run line triggers the cases?) If so then there really should be a neon specific test file that verifies the widening and scalarisation changes as the NEON run line for the "fixed-length" tests is more about ensuring no SVE instructions slip through.

Thanks, Paul. No rush at all. Digressing a bit, we should probably sync-up on what else needs to be done for fixed length lowering. I believe this is the last operation on my list.

The legalisation changes are specific to VECREDUCE_SEQ_FADD on SVE. Prior to this patch, VECREDUCE_SEQ_FADD is expanded in the ExpandReductions pass before legalisation runs, so VECREDUCE_SEQ_FADD would never reach legalisation on NEON. (That's assuming NEON has no FADDA support. I might be wrong about that.)

The new tests would be broken without the legalisation changes, so I'm assuming that those are enough coverage. Maybe I'm missing something though...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89162/new/

https://reviews.llvm.org/D89162