[PATCH] D13988: [X86][SSE] Add general memory folding for (V)INSERTPS instruction

[Filipe Cabecinhas via llvm-commits]

filcab added a comment.

LGTM, but I'm not the most well versed in the MachineInstr stuff.
Thanks for making the filecheck stuff match more of the instruction, too!


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26651
@@ -26696,3 +26650,3 @@
   case X86ISD::VPERM2X128:
-  case ISD::VECTOR_SHUFFLE: return PerformShuffleCombine(N, DAG, DCI,Subtarget);
+  case ISD::VECTOR_SHUFFLE: return PerformShuffleCombine(N, DAG, DCI, Subtarget);
   case ISD::FMA:            return PerformFMACombine(N, DAG, Subtarget);
----------------
Please split the clang-format (or manual format) changes from the rest.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4894
@@ -4871,1 +4893,3 @@
+  MachineInstr *NewMI =
+      MF.CreateMachineInstr(TII.get(Opcode), MI->getDebugLoc(), true);
   MachineInstrBuilder MIB(MF, NewMI);
----------------
Please split the clang-format changes from the rest.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4923
@@ -4898,1 +4922,3 @@
 
+MachineInstr *X86InstrInfo::foldMemoryOperandSpecial(
+    MachineFunction &MF, MachineInstr *MI, unsigned OpNum,
----------------
I would s/Special/Custom/, but that's just me bikeshedding :-)

================
Comment at: lib/Target/X86/X86InstrInfo.h:515
@@ -514,1 +514,3 @@
 
+  /// Handles memory folding for special case instructions, e.g. those requiring
+  /// custom manipulation of the address.
----------------
Can you double-check the doxygen that gets generated? IIRC, it stops at the first '.'.

Unless there's a special case for "e.g.", it's probably best to replace it with a more colloquial "like", or "for example:".


Repository:
  rL LLVM

http://reviews.llvm.org/D13988