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

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 17 20:26:09 PDT 2023


goldstein.w.n added a comment.

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



================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:3849
+    const MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+    MovMI = MRI.getVRegDef(MovReg);
+  }
----------------
This is a big patch. I would split.tgis edge case to a follow up and just fail in this case for now.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4792
+  if (UseMI.getMF()->getFunction().hasOptSize() && Reg.isVirtual() &&
+      !MRI->hasOneNonDBGUse(Reg))
+    return false;
----------------
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.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4906
+    break;
+  case X86::SAR32rCL:
+    NewOpc = X86::SAR32ri;
----------------
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.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4910
+    break;
+  default:
+    return false;
----------------
Is there really no helper for getting immediate opcide from rr version?


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

https://reviews.llvm.org/D151848



More information about the llvm-commits mailing list