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

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/CodeGen/PeepholeOptimizer.cpp:1357
     DenseMap<Register, MachineInstr *> &ImmDefMIs) {
   const MCInstrDesc &MCID = MI.getDesc();
+  if (MCID.getNumDefs() != 1 || !MI.getOperand(0).isReg())
----------------
The code looks cleaner with early return like before:

if (!MI.isMoveImmediate() && !TII->getConstValDefinedInReg(MI,Reg,ImmVal))
    return false;


================
Comment at: llvm/lib/CodeGen/PeepholeOptimizer.cpp:1358
   const MCInstrDesc &MCID = MI.getDesc();
-  if (!MI.isMoveImmediate())
-    return false;
-  if (MCID.getNumDefs() != 1)
+  if (MCID.getNumDefs() != 1 || !MI.getOperand(0).isReg())
     return false;
----------------
Should this be

if (MI.isMoveImmediate() && MCID.getNumDefs() != 1 
   || !MI.isMoveImmediate() && !MI.getOperand(0).isReg())
   return false;


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4895
+
+/// Real implementation of FoldImmediate.
+bool X86InstrInfo::FoldImmediateImpl(MachineInstr &UseMI, MachineInstr *DefMI,
----------------
document parameters and return.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4967
+  if ((NewOpc == X86::SUB64ri32 || NewOpc == X86::SUB32ri) &&
+      UseMI.findRegisterUseOperandIdx(Reg) != 2)
+    return false;
----------------
brief commend here.


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

https://reviews.llvm.org/D151848