[PATCH] Loop Rerolling Pass

Andrew Trick atrick at apple.com
Mon Oct 28 15:40:16 PDT 2013


  Why are you looking for Phi-IVs across all loop blocks instead of just the header block in collectPossibleIVs and collectPossibleReductions?

  Why is collectPossibleReductions a separate pass from collectPossibleIVs?

  I'm not sure what IVInfo is for. It looks like the SCEV field is always the result of calling getSCEV on the first field.

  I'm not sure why findScaleFromMul needs to look for Mul instructions instead of examining SCEVs.

  I don't know what "Final" is for.

  Eventually I had to punt on the review. I wasn't able to grok everything that is going on in LoopReroll::reroll. The function is > 400 lines, with lots of state. Since I don't really understand the algorithm, it's hard for me to suggest a better way of doing it. If the results are good and compilation time isn't terrible, I can't argue against checking it in as a prototype. It needs a lot of improvement though before I would consider it reviewable/maintainable.


================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:215
@@ +214,3 @@
+        if (const SCEVConstant *IncSCEV =
+            dyn_cast<SCEVConstant>(PHISCEV->getOperand(1))) {
+          if (!IncSCEV->getValue()->getValue().isStrictlyPositive())
----------------
Did you mean PHISCEV->getStepRecurrence()


http://llvm-reviews.chandlerc.com/D1940



More information about the llvm-commits mailing list