[PATCH] D16829: An implementation of Swing Modulo Scheduling

[Brendon Cahoon via llvm-commits]

bcahoon added inline comments.

================
Comment at: include/llvm/Target/TargetInstrInfo.h:1049
@@ -1012,1 +1048,3 @@
 
+  /// Return true for post-incremented instructions.
+  virtual bool isPostIncrement(const MachineInstr* MI) const {
----------------
marksl wrote:
> Should this return true for pre & post increment & decrement?
I've only tested this for post increment.  I think returning true for the other cases would cause problems, since the offset value adjustment would probably be incorrect (at least for the post/pre decrement cases). The pre-increment case would end up ok.  A more general solution could be useful if your target, or others, have these variants. The pipeliner uses this information to eliminate the dependence on the post incremented value w.r.t other instructions that use the same base value.

================
Comment at: lib/CodeGen/MachinePipeliner.cpp:877
@@ +876,3 @@
+
+  if (!L->getLoopPreheader())
+    return false;
----------------
marksl wrote:
> I've seen CodeGenPrepare delete the preheader. Specifically, if the previous block is a loop and this loop immediate follows it. I don't have a test case, but it's basically two sequential for loops. I wonder if that has any impact here?
Yes, it would have an impact for loops without a preheader.  On Hexagon, when we create a hardware loop, the preheader is added if it's not there already.  Then, the pipeliner pass only sees loops with a preheader.  It would be easy enough to add a preheader if one doesn't exist already.


http://reviews.llvm.org/D16829