[PATCH] D14762: X86-FMA3: Memory folding for scalar loads + FMA3

[David Kreitzer via llvm-commits]

DavidKreitzer added a comment.

Hi Slava,

Everything looks straightforward to me.  I just have a few minor comments.

Thanks,
-Dave


================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:164
@@ -163,3 +163,3 @@
 // the scalar FMA intrinsics are lowered to machine instructions, and in that
 // sence they are similar to existing ADD*_Int, SUB*_Int, MUL*_Int, etc.
 // instructions.
----------------
Just noticed a few typos in this comments while I was reviewing the code.

sence --> sense

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:173
@@ -172,3 +172,3 @@
 // the lowest element of the FMA*_Int instruction. Even though such analysis
 // may be not implemened yet we allow the routines doing the actual commute
 // transformation to decide if one or another instruction is commutable or not.
----------------
implemened --> implemented

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:245
@@ -254,1 +244,3 @@
+            (COPY_TO_REGCLASS(!cast<Instruction>(NAME#"SDr213r_Int") 
+             $src1, $src2, $src3),VR128)>;
 }
----------------
Please add a space after ','

================
Comment at: llvm/test/CodeGen/X86/fma-scalar-memfold.ll:1
@@ +1,2 @@
+; RUN: llc < %s -mtriple=x86_64-pc-win32 -mcpu=core-avx2 | FileCheck %s
+
----------------
I like the thoroughness of your test!

A couple ideas to make the test a little less sensitive to innocuous changes.

(1) You could avoid checking for the block labels, e.g. "# BB#0" and just change the subsequent CHECK-NEXT to CHECK.
(2) You could avoid explicitly checking for xmm0 and instead use a variable.


================
Comment at: llvm/test/CodeGen/X86/fma-scalar-memfold.ll:5
@@ +4,3 @@
+
+declare <4 x float> @llvm.x86.fma.vfmadd.ss(<4 x float>, <4 x float>, <4 x float>) #3
+declare <4 x float> @llvm.x86.fma.vfmsub.ss(<4 x float>, <4 x float>, <4 x float>) #3
----------------
"#3" is not defined.


http://reviews.llvm.org/D14762