[PATCH] D156614: [AArch64][GISel] Handling for G_VECREDUCE_FMIN and G_VECREDUCE_FMAX

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D156614#4545187 <https://reviews.llvm.org/D156614#4545187>, @tschuett wrote:

> You could argue that the assembler show that importing works as expected and you find `fmaxnmv` now. But they are also end-to-end tests. They go from LLVM IR to assembler while visiting the IR translator, the legalizer, regbandk select, and maybe instruction selection. A legalize-vecreduce.mir with MIR tests might help you to prove that your legalization strategy works in isolation. IDK how far you can go with a select-vecreduce.mir to test the selection in isolation.

Yeah... And in my opinion it is the end-to-end tests that are most useful. It's worked out well for all the existing selectiondag tests like these that can be kept simple. Unfortunately it can be all too easy (and has happened with some of the existing tests) where the mir test a small portion is instruction selection without the whole pipeline working together. The mir test can be useful for testing special cases, especially in cases like https://reviews.llvm.org/D157202 where they can test something in (relative) isolation.  But in cases like this the llc tests are providing decent test coverage, in a way that is fairly clear from assembly. I'm not sure I see the advantage of a lot of boilerplate tests, it feels like unnecessary busywork.

So I believe this has test coverages. If there are any specific cases that are worth adding as mir tests then let me know and I can add them.


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

https://reviews.llvm.org/D156614