[PATCH] D14550: X86-FMA3: Implemented commute transformations for FMA*_Int instructions

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a reviewer: qcolombet.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Slava,

LGTM with a couple of comments.
Feel free to commit whenever you want.

Cheers,
-Quentin


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:3547
@@ -3468,1 +3546,3 @@
+  // The input opcode does not match with any of the opcodes from the tables.
+  if (FoundOpcodesGroup == nullptr)
     return 0;
----------------
Good point!
Turn that into an assert maybe?
That way, if it happens, we would know there are things that can be improved :).

Up to you.

================
Comment at: llvm/test/CodeGen/X86/fma-commute-x86.ll:12
@@ +11,3 @@
+; CHECK-NEXT: vmovap{{s|d}} {{\(%rdx\), %xmm0|\(%rcx\), %xmm1}}
+; CHECK-NEXT: vmovap{{s|d}} {{\(%rdx\), %xmm0|\(%rcx\), %xmm1}}
+; CHECK-NEXT: vfmadd213ss %xmm1, %xmm1, %xmm0
----------------
I am guessing you put regular expression here because you want the test to be robust against scheduling changes. This is usually not the way we go, i.e., we tend to use the DAG construct for such cases. That being said, I do not remember whether DAG support NEXT. Anyway, for now, we may just match the actual scheduling, unless you saw it changing?


http://reviews.llvm.org/D14550