[PATCH] D143787: [X86] Add new pass `X86FixupISel` for fixing up machine-instruction selection.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FixupISel.cpp:12
+// attempting to get the "right" instruction can break patterns. This pass
+// is not meant to do any analysis. It is only meant to be make transformations
+// that are guaranteed to be doable.
----------------
What do you mean by analysis? I'm hoping we can use scheduler models in the future to drive some transforms here.


================
Comment at: llvm/lib/Target/X86/X86FixupISel.cpp:68
+  unsigned NumOperands = MI.getDesc().getNumOperands();
+
+  auto processVPERMILPSm = [&](unsigned NewOpc) {
----------------
Comments explaining the exact purpose of these transforms would be useful.


================
Comment at: llvm/lib/Target/X86/X86FixupISel.cpp:69
+
+  auto processVPERMILPSm = [&](unsigned NewOpc) {
+    if (!ST->hasNoDomainDelayShuffle())
----------------
(style) use bool instead of auto


================
Comment at: llvm/lib/Target/X86/X86FixupISel.cpp:1
+//===-- X86FixupISels.cpp - use or replace ISel instructions -----------===//
+//
----------------
pengfei wrote:
> craig.topper wrote:
> > Referring to ISel and running the pass so far from instruction selection is perhaps misleading.
> I have the same concern, if it is required for `vpermli*`, maybe just name it `FixupVpermli`.
The hope is to add other instruction combos in the future - given we're trying to fixup for a mixture of tuning/scheduler - maybe X86FixupTuning.cpp ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143787