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

[Vyacheslav Klochkov via llvm-commits]

v_klochkov added a comment.

Thank you for the review!

I updated the unit test.
Undefined ExeDomain for scalar FMAs is the problem unrelated to the one fixed by this patch (memory folding for FMA*_Int). So, it should be fixed in a separate patch. I can prepare such patch later.

Thank you,
Slava


================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:233
@@ -232,3 +232,3 @@
             VEX_W;
 
   // These patterns use the 123 ordering, instead of 213, even though
----------------
RKSimon wrote:
> Please would it be possible to add ExecutionDomains to these definitions? For some reason only the packed FMA instructions have Single/Double domains set. 
Good catch, thank you for the comment!
That would be a simple additional fix, but I want to follow the recommendation: "1 patch fixes 1 problem", 
I.e. I can set the ExeDomain for scalar FMAs in a separate patch.

================
Comment at: llvm/test/CodeGen/X86/fma-scalar-memfold.ll:17
@@ +16,3 @@
+; CHECK-LABEL: fmadd_aab_ss:
+; CHECK:      # BB#0:
+; CHECK-NEXT: vmovss (%rcx), %[[XMM:xmm[0-9]+]]
----------------
DavidKreitzer wrote:
> This isn't quite what I meant about the block labels.  I think you should just delete line 17 here and change line 18 "CHECK-NEXT" --> "CHECK". That way, if # BB#0 changes to something else, it won't affect this test.
> 
> The %[[XMM]] changes are great, thanks!
> 
Ok, understood, I'll fix the checks.


http://reviews.llvm.org/D14762