[PATCH] D13269: Improved X86-FMA3 mem-folding & coalescing

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:37
@@ -19,1 +36,3 @@
+
+let Constraints = "$src1 = $dst", hasSideEffects = 0, isCommutable = 1 in
 multiclass fma3p_rm<bits<8> opc, string OpcodeStr,
----------------
Why are you changing the hasSideEffects semantic here?
I am not saying it is false, it just seems out of scope of what is necessary for this patch. What I am missing?

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:57
@@ -56,3 +74,2 @@
 }
-} // Constraints = "$src1 = $dst"
 
----------------
Although that is not incorrect, why do you get rid of that {?
You did the same later in that patch. Again, this is not wrong, but this kind of clean-ups adds noise to the review. I.e., if you want to do such clean-ups, which I am not sure I agree, that should be a separate patch.

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:162
@@ -161,3 +160,2 @@
 }
-} // Constraints = "$src1 = $dst", hasSideEffects = 0
 
----------------
Ditto.

================
Comment at: llvm/lib/Target/X86/X86InstrFMA.td:194
@@ -193,3 +192,1 @@
-} // Constraints = "$src1 = $dst", isCommutable = 0, isCodeGenOnly =1,
-  // hasSideEffects = 0
 
----------------
Ditto.


http://reviews.llvm.org/D13269