[PATCH] D123995: [MachineSink] replace MachineLoop with MachineCycle

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds accepted this revision.
sameerds added a comment.
This revision is now accepted and ready to land.

The changes in GenericCycleInfo look good to me. But please do wait for reviewers looking at MachineSink.



================
Comment at: llvm/include/llvm/ADT/GenericCycleInfo.h:132
+  /// Return the preheader block for this cycle. This only works for reducible
+  /// cycles for now.
+  BlockT *getCyclePreheader() const;
----------------
Instead of saying "for now", we could simply say that pre-header is well-defined for reducible cycles, as listed in LoopTerminology.html. Then also add "returns nullptr if irreducible".

Same for the next function.


================
Comment at: llvm/include/llvm/CodeGen/MachineCycleAnalysis.h:58
+
+// TODO: add this function to MachineCycle template after implementing IR
+//       version.
----------------
I think you mean "add to GenericCycle"?


================
Comment at: llvm/include/llvm/CodeGen/MachineSSAContext.h:31
 inline auto predecessors(MachineBasicBlock *BB) { return BB->predecessors(); }
+inline auto succ_size(MachineBasicBlock *BB) { return BB->succ_size(); }
+inline auto pred_size(MachineBasicBlock *BB) { return BB->pred_size(); }
----------------
Oops! Thanks for adding this to trunk! Now I realize I was looking at a topic branch in my repo where I had already put these in place. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123995