[PATCH] D85744: [SVE] Lower fixed length FP minnum/maxnum

Tue Aug 11 12:25:57 PDT 2020

cameron.mcinally marked 2 inline comments as done.
cameron.mcinally added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-fp-minmax.ll:558
+
+; NOTE: Check lines only cover the first VBYTES because the fmaxnm_v#f16 tests
+; already cover the general legalisation cases.
----------------
paulwalker-arm wrote:
> I'm happy with the tests, but just wanted to raise awareness of the "other" style of writing tests as shown by D85724.
> 
> Without going silly with covering all possible cases there's two options.  Stronger type legalisation tests as used by this patch and stronger "end result" testing as used by D85724.
> 
> Both are equally valid, it just comes down to whether you think type legalisation is already pretty much covered by other instruction tests and possible also which allows for better protection.
> 
> As I say, I'm not expecting any changes here, I'm just presenting the options.
> 
Ah, ok. Makes sense. I had originally based the tests off of FADD, so that's where I got the legalization style from. Updated to the end result style now.

Hopefully I didn't make any mistakes. Updating all these fixed VLs is mind numbing...

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

https://reviews.llvm.org/D85744