[PATCH] D64665: [MachinePipeliner] Refactor schedule emission logic

[Jinsong Ji via Phabricator via llvm-commits]

jsji added a comment.

Personally, I like the idea of refacting and more abstraction, 
but I am a little worry about it might nont handle the complication of Phi well.
And unfortunately, I don't know enough about the cases here, especially for Hexagon.
So I would like to hear the valuable feedback from Brendon @bcahoon as well.

BTW: I am seeing Assertion failures in `sms-grp-order.ll`:

  Assertion `isVirtualRegister(Reg) && "Not a virtual register"' failed

And also quite a few other Assertion when running test with test-suites.



================
Comment at: include/llvm/CodeGen/MachinePipeliner.h:712
+  // The index of this stage.
+  unsigned StageIdx;
+  // The inputs to this stage. The last input (if Inputs.size() == 2) may be
----------------
private field 'StageIdx' is not used 


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:2279
     return true;
-
+  dbgs() << "probably not\n";
   return (OffsetS + (int64_t)AccessSizeS < OffsetD + (int64_t)AccessSizeD);
----------------
debug output


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:3161
+  std::deque<Stage *> KernelStages(1, nullptr);
+  for (int I = 1; I < NumStages; ++I)
+    KernelStages.push_back(
----------------
comparison of integers of different signs: 'int' and 'unsigned int'


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:3165
+  KernelStages[0] = KernelPB.CreateStage({KernelStages[1]}, 0);
+  for (int I = 1; I < NumStages; ++I)
+    KernelStages[I]->setLastInput(KernelStages[I - 1]);
----------------
comparison of integers of different signs: 'int' and 'unsigned int'


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:3200
+    // stage to force PHI creation.
+    for (unsigned I = 0; I < PeelStage - 1; ++I)
+      EpilogStages[I] = PB.CreateStage({PrologStages[I], EpilogStages[I]}, ~1U);
----------------
comparison of integers of different signs: 'unsigned int' and 'int'


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:3206
+    Prologs[PeelStage - 1].addSuccessors(
+        PB, PeelStage == NumStages - 1 ? KernelPB : Prologs[PeelStage]);
+    if (!Epilogs.empty()) {
----------------
comparison of integers of different signs: 'int' and 'unsigned int'


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64665