[PATCH] D13710: New X86 FMA3*_Int opcodes for scalar FMA intrinsics.

[Vyacheslav Klochkov via llvm-commits]

v_klochkov added a comment.

Hi Quentin,

Thank you for the comment. I would rather not adding additional checks to that test, 
but would remove/cancel the scalar test cases I added to fma-intrinsics-phi-213-to-231.ll test.
Please see see more details in my answer to your inline comment.

Thank you,
Slava


================
Comment at: llvm/test/CodeGen/X86/fma-intrinsics-phi-213-to-231.ll:4
@@ +3,3 @@
+; CHECK-LABEL: fmaddsd_loop:
+; CHECK:   vfmadd213sd        %xmm{{[0-9]+}}, %xmm{{[0-9]+}}, %xmm{{[0-9]+}}
+define <2 x double> @fmaddsd_loop(i32 %iter, <2 x double> %a, <2 x double> %b, <2 x double> %c) {
----------------
qcolombet wrote:
> Could you add more check lines to check that the registers are set to the right value?
> This just this check I guess that previously buggy code would also match and we don’t want that.
The test cases in this LIT test are quite complex, and adding such additional checks should be quite complex and it may
make the test unstable, pass/fail may depend on many optimizations working on the tested loops. 

The initial idea of the test fma-intrinsics-phi-213-to-231.ll _probably_ was only
to check if the corresponding PHI-213-to-231 optimization did its work for FMAs with ADD accumulators.
The idea of my changes was to check that the optimization 
DID NOT do changes for SCALAR FMA instructions generated for scalar FMA intrinsics.

Your guess that the new scalar test case would pass all checks even without my new patch is correct.
I did an additional investigation and discovered that PHI-213-to-231 optimization never did any changes
for scalar FMA instructions generated for intrinsics because of the additional REG copies inserted
in the pattern (see the base version of the file X86InstrFMA.td the lines 194-196).

After giving it some more thought I think that the scalar test cases I added to this test are
redundant as PHI-213-to-231 optimization is just a special instruction inserter which is never
called for the new FMA*_Int instructions.

The other aspects/transformations like commuting operands should be checked by other specialized tests
like fma-commutex86.ll added in (D13269).

Do you agree with my plan to remove the scalar cases from the test?
Also, it may be useful to add test cases for 128-bit packed FMAs to this test, 
I just surprisingly realized that the test has only 256-bit test cases in it.



http://reviews.llvm.org/D13710