[PATCH] D93694: [MachineLICM][MachineSink] Move SinkIntoLoop from MachineLICM to MachineSink

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:86
+                           "register spills"),
+                  cl::init(false), cl::Hidden);
+
----------------
Is there a plan to add an option for the maximum number of instructions sunk, or something like that?


================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:363
+void MachineSinking::FindLoopSinkCandidates(MachineLoop *L, MachineBasicBlock *BB,
+    SmallVector<MachineInstr *, 8> &Candidates) {
+  for (auto &MI : *BB) {
----------------
SmallVectorImpl<MachineInstr*>&


================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:387
+      continue;
+    if (!MRI->hasOneDef(MO.getReg()))
+      continue;
----------------
What are we doing these checks for?


================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:1171
+
+  for (MachineInstr &MI : MRI->use_instructions(MO.getReg())) {
+    LLVM_DEBUG(dbgs() << "LoopSink:   Analysing use: "; MI.dump());
----------------
So how do we know that MI is in a loop?


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

https://reviews.llvm.org/D93694