[LV] Minor fixes to Sink casts to unravel first order recurrence

[Ayal]

Summary

Two minor fixes: avoid copying a map and avoid redundant motion.

[mssimpso]

lib/Transforms/Utils/LoopUtils.cpp

568 ↗

(On Diff #110018)

Is it worth adding a test case for this? I'm not sure...

[Ayal]

Is it worth adding a test case for this? I'm not sure...

Can add a check that a cast wasn't moved needlessly, as in the augmented version uploaded.

[mssimpso]

LGTM.

llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp

bool RecurrenceDescriptor::isFirstOrderRecurrence(
   // The dominance requirement ensures the loop vectorizer will not need to
   // vectorize the initial value prior to the first iteration of the loop.
   // TODO: Consider extending this sinking to handle other kinds of instructions
   // and expressions, beyond sinking a single cast past Previous.
   if (Phi->hasOneUse()) {
     auto *I = Phi->user_back();
     if (I->isCast() && (I->getParent() == Phi->getParent()) && I->hasOneUse() &&
         DT->dominates(Previous, I->user_back())) {
+      if (!DT->dominates(Previous, I)) // Otherwise we're good w/o sinking.
         SinkAfter[I] = Previous;
       return true;
     }
   }
 
   for (User *U : Phi->users())
     if (auto *I = dyn_cast<Instruction>(U)) {
       if (!DT->dominates(Previous, I))
         return false;

llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp

void LoopVectorizationPlanner::executePlan(InnerLoopVectorizer &ILV) {
   // into the code below should also be implemented in
   // the cost-model.
   //
   //===------------------------------------------------===//
 
   // 2. Copy and widen instructions from the old loop into the new loop.
 
   // Move instructions to handle first-order recurrences.
-  DenseMap<Instruction *, Instruction *> SinkAfter = Legal->getSinkAfter();
+  DenseMap<Instruction *, Instruction *> &SinkAfter = Legal->getSinkAfter();
   for (auto &Entry : SinkAfter) {
     Entry.first->removeFromParent();
     Entry.first->insertAfter(Entry.second);
     DEBUG(dbgs() << "Sinking" << *Entry.first << " after" << *Entry.second
                  << " to vectorize a 1st order recurrence.\n");
   }
 
   // Collect instructions from the original loop that will become trivially dead

llvm/trunk/test/Transforms/LoopVectorize/first-order-recurrence.ll

 ; }
 ;
 ; CHECK-LABEL: @recurrence_3(
 ; CHECK:       vector.ph:
 ; CHECK:         %vector.recur.init = insertelement <4 x i16> undef, i16 %0, i32 3
 ; CHECK:       vector.body:
 ; CHECK:         %vector.recur = phi <4 x i16> [ %vector.recur.init, %vector.ph ], [ [[L1:%[a-zA-Z0-9.]+]], %vector.body ]
 ; CHECK:         [[L1]] = load <4 x i16>
-; CHECK:         {{.*}} = shufflevector <4 x i16> %vector.recur, <4 x i16> [[L1]], <4 x i32> <i32 3, i32 4, i32 5, i32 6>
+; CHECK:         [[SHUF:%[a-zA-Z0-9.]+]] = shufflevector <4 x i16> %vector.recur, <4 x i16> [[L1]], <4 x i32> <i32 3, i32 4, i32 5, i32 6>
+; Check also that the casts were not moved needlessly.
+; CHECK:         sitofp <4 x i16> [[L1]] to <4 x double>
+; CHECK:         sitofp <4 x i16> [[SHUF]] to <4 x double> 
 ; CHECK:       middle.block:
 ; CHECK:         %vector.recur.extract = extractelement <4 x i16> [[L1]], i32 3
 ; CHECK:       scalar.ph:
 ; CHECK:         %scalar.recur.init = phi i16 [ %vector.recur.extract, %middle.block ], [ %0, %vector.memcheck ], [ %0, %for.preheader ]
 ; CHECK:       scalar.body:
 ; CHECK:         %scalar.recur = phi i16 [ %scalar.recur.init, %scalar.ph ], [ {{.*}}, %scalar.body ]
 ;
 ; UNROLL-LABEL: @recurrence_3(