[PATCH] D151848: [X86, Peephole] Enable FoldImmediate for X86

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

The peephole optimizer changes look good to me.
+1 on every comments touching on test cases and comments.

For the folding of opc, it may be interesting to use a similar approach as the `MemoryFoldTableX`. This uses an X86 specific tablegen backend. (See `llvm/utils/TableGen/X86FoldTablesEmitter.cpp`.)

I only had a cursory look at the X86 changes and they looked fine. The fold table code is quite big though. I leave that part to x86 folks to approve.



================
Comment at: llvm/lib/CodeGen/PeepholeOptimizer.cpp:223
+                           DenseMap<RegSubRegPair, MachineInstr *> &CopyMIs,
+                           SmallPtrSetImpl<MachineInstr *> &LocalMIs);
 
----------------
davidxl wrote:
> add documentation on the new parameter LocalMIs
+1 without the comment it is impossible to understand why we would skip these instructions.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.h:554
+
+  /// FoldImmediate - 'Reg' is known to be defined by a move immediate
+  /// instruction, try to fold the immediate into the use instruction.
----------------
Don't repeat the function name in the comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151848/new/

https://reviews.llvm.org/D151848