[PATCH] Force RerollLoop pass to reset the loop trip count value for rerolled loops

[zinovy.nis]

Summary

It's a bug fix.
For rerolled loops SE trip count remains unchanged. It leads to incorrect work of the next passes.

My patch just resets SE info for rerolled loop forcing SE to re-evaluate it next time it requested.

I also added a verifier call in the exisitng test to be sure no invalid SE data remain. Without my fix this test would fail with -verify-scev.

[Ayal]

Nice catch. Looks good to me. Just to clarify, the testcase currently fails with -verify-scev, right?

[Ayal]

You may want to add Hal Finkel (if others don't review); he seems to have last touched that code according to svn blame ;-) (And James Molloy claimed last week to have rewritten it).

[zinovy.nis]

In D18316#379451, @Ayal wrote:

the testcase currently fails with -verify-scev, right?

Yes, it fails.

[jmolloy]

LGTM too, thanks for this!

llvm/trunk/lib/Transforms/Scalar/LoopRerollPass.cpp

bool LoopReroll::runOnLoop(Loop *L, LPPassManager &LPM) {
   DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);
 
   BasicBlock *Header = L->getHeader();
   DEBUG(dbgs() << "LRR: F[" << Header->getParent()->getName() <<
         "] Loop %" << Header->getName() << " (" <<
         L->getNumBlocks() << " block(s))\n");
 
-  bool Changed = false;
-
   // For now, we'll handle only single BB loops.
   if (L->getNumBlocks() > 1)
-    return Changed;
+    return false;
 
   if (!SE->hasLoopInvariantBackedgeTakenCount(L))
-    return Changed;
+    return false;
 
   const SCEV *LIBETC = SE->getBackedgeTakenCount(L);
   const SCEV *IterCount = SE->getAddExpr(LIBETC, SE->getOne(LIBETC->getType()));
   DEBUG(dbgs() << "LRR: iteration count = " << *IterCount << "\n");
 
   // First, we need to find the induction variable with respect to which we can
   // reroll (there may be several possible options).
   SmallInstructionVector PossibleIVs;
   IVToIncMap.clear();
   collectPossibleIVs(L, PossibleIVs);
 
   if (PossibleIVs.empty()) {
     DEBUG(dbgs() << "LRR: No possible IVs found\n");
-    return Changed;
+    return false;
   }
 
   ReductionTracker Reductions;
   collectPossibleReductions(L, Reductions);
+  bool Changed = false;
 
   // For each possible IV, collect the associated possible set of 'root' nodes
   // (i+1, i+2, etc.).
   for (SmallInstructionVector::iterator I = PossibleIVs.begin(),
        IE = PossibleIVs.end(); I != IE; ++I)
     if (reroll(*I, L, Header, IterCount, Reductions)) {
       Changed = true;
       break;
     }
 
+  // Trip count of L has changed so SE must be re-evaluated.
+  if (Changed)
+    SE->forgetLoop(L);
+
   return Changed;
 }

llvm/trunk/test/Transforms/LoopReroll/basic32iters.ll

-; RUN: opt < %s -loop-reroll -S | FileCheck %s
+; RUN: opt < %s -loop-reroll -verify-scev -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; void goo32(float alpha, float *a, float *b) {
 ;   for (int i = 0; i < 3200; i += 32) {
 ;     a[i] += alpha * b[i];
 ;     a[i + 1] += alpha * b[i + 1];
 ;     a[i + 2] += alpha * b[i + 2];