[PATCH] D148980: [X86] Machine combine vnni instruction.

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCombiner.cpp:173
+    }
+  }
   // PHI's have no depth etc.
----------------
goldstein.w.n wrote:
> What is this change for?
For some reason the new created register for new created instructions is not managed by MRI, so it would miss adding the DefInstr's latency. I guess may be due to the new instructions are not inserted in the machine function. I'll debug more on it.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:9822
+      DelInstrs.push_back(&Root);
+      break;
+    }
----------------
goldstein.w.n wrote:
> Maybe this belongs in `X86FixupInstTuning.cpp`?
The method is to override the virtual function of TargetInstrInfo. 
X86FixupInstTuning.cpp may be integrated to MachineCombine.


================
Comment at: llvm/test/CodeGen/X86/avxvnni-combine.ll:4
+
+; __m256i foo(int cnt, __m256i c, __m256i b, __m256i *p) {
+;     for (int i = 0; i < cnt; ++i) {
----------------
goldstein.w.n wrote:
> Can you precommit the test so we can see the diff?
Sure, I'll precommit the test case first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148980