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

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added a comment.

In D151848#4597644 <https://reviews.llvm.org/D151848#4597644>, @goldstein.w.n wrote:

> Some codesize/Reg slloc stats from llvm test suite would be useful to have for evaluating the benefit of this patch.

I will collect these stats for spec int 2006.



================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:3849
+    const MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+    MovMI = MRI.getVRegDef(MovReg);
+  }
----------------
goldstein.w.n wrote:
> This is a big patch. I would split.tgis edge case to a follow up and just fail in this case for now.
All instructions loading constant to 64bit register have this pattern.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4792
+  if (UseMI.getMF()->getFunction().hasOptSize() && Reg.isVirtual() &&
+      !MRI->hasOneNonDBGUse(Reg))
+    return false;
----------------
goldstein.w.n wrote:
> Does this actually payoff? Do we ever spills and not just rematerialize the constant op in RA?
> Because if not, saving code side for the perf benefit may win out for many of these.
Interesting question, I haven't studied reload vs rematerialization in RA.
But I prefer rematerialization for constant in RA, because if we reload the constant from memory we need a [sp+offset] operand, the offset field is also a 32bit immediate, why not use the bytes to directly rematerialize a register, and save a memory load operation.
So here we keep a constant in register if it is used multiple times to save size. And later if we experience high register pressure, it can be rematerialized on demand.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4906
+    break;
+  case X86::SAR32rCL:
+    NewOpc = X86::SAR32ri;
----------------
goldstein.w.n wrote:
> Personally would split patch and in the first iteration do only shifts and zero. Those are obviously beneficial cases. The rest are a bit less clear.
Actually the motivating case is AND and ADD, as shown in https://reviews.llvm.org/D119916#change-nKHV7D4KKS4t. It is extracted from tcmalloc.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4910
+    break;
+  default:
+    return false;
----------------
goldstein.w.n wrote:
> Is there really no helper for getting immediate opcide from rr version?
I couldn't find one.


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

https://reviews.llvm.org/D151848