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

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

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

Thanks very much for review @sameerds . Sure, I will wait for the MachineSink part review.



================
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;
----------------
sameerds wrote:
> 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.
Updated.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123995