[IndVars] Remove unreasonable checks in rewriteLoopExitValues

[mkazantsev]

Summary

A piece of logic in rewriteLoopExitValues has a weird check on number of
users which allowed an unprofitable transform in case if an instruction has
more than 6 users.

[mkazantsev]

Ping?

[mkazantsev]

Rebased.

[etherzhhb]

LGTM

lib/Transforms/Scalar/IndVarSimplify.cpp

616–631 ↗

(On Diff #165718)

The only suggestion is maybe we could extract this piece of code to a standalone function.
This can make the code easier to understand

[mkazantsev]

lib/Transforms/Scalar/IndVarSimplify.cpp

616–631 ↗

(On Diff #165718)

It's done in the follow-up patch https://reviews.llvm.org/D51584. I'm merging them one by one just to make reviewers' life easier. :)

[etherzhhb]

lib/Transforms/Scalar/IndVarSimplify.cpp

616–631 ↗

(On Diff #165718)

Is there other patches between this and D51584? I saw some thing like "if (ExitValue->getSCEVType()>=scMulExpr) {" is gone (just out of curiosity :) )

[mkazantsev]

lib/Transforms/Scalar/IndVarSimplify.cpp

616–631 ↗

(On Diff #165718)

Yes there is, https://reviews.llvm.org/D51581

You can use "Stack" tab in Revision Contensts section to see dependencies. :)

As for this check, it's removed in the last patch (I really have no idea why it was there).

[etherzhhb]

lib/Transforms/Scalar/IndVarSimplify.cpp

616–631 ↗

(On Diff #165718)

I see

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

while ((PN = dyn_cast<PHINode>(BBI++))) {
         // Computing the value outside of the loop brings no benefit if :
         //  - it is definitely used inside the loop in a way which can not be
         //    optimized away.
         //  - no use outside of the loop can take advantage of hoisting the
         //    computation out of the loop
         if (ExitValue->getSCEVType()>=scMulExpr) {
           bool HasHardInternalUses = false;
           bool HasSoftExternalUses = false;
-          unsigned NumUses = 0;
-          for (auto IB = Inst->user_begin(), IE = Inst->user_end();
-               IB != IE && NumUses <= 6; ++IB) {
-            Instruction *UseInstr = cast<Instruction>(*IB);
+          for (auto *IB : Inst->users()) {
+            Instruction *UseInstr = cast<Instruction>(IB);
             unsigned Opc = UseInstr->getOpcode();
-            NumUses++;
             if (L->contains(UseInstr)) {
               if (Opc == Instruction::Call)
                 HasHardInternalUses = true;
             } else {
               if (Opc == Instruction::PHI) {
                 // Do not count the Phi as a use. LCSSA may have inserted
                 // plenty of trivial ones.
-                NumUses--;
-                for (auto PB = UseInstr->user_begin(),
-                          PE = UseInstr->user_end();
-                     PB != PE && NumUses <= 6; ++PB, ++NumUses) {
-                  unsigned PhiOpc = cast<Instruction>(*PB)->getOpcode();
+                for (auto *PB : UseInstr->users()) {
+                  unsigned PhiOpc = cast<Instruction>(PB)->getOpcode();
                   if (PhiOpc != Instruction::Call &&
                       PhiOpc != Instruction::Ret) {
                     HasSoftExternalUses = true;
                     break;
                   }
                 }
                 continue;
               }
               if (Opc != Instruction::Call && Opc != Instruction::Ret) {
                 HasSoftExternalUses = true;
                 break;
               }
             }
           }
-          if (NumUses <= 6 && HasHardInternalUses && !HasSoftExternalUses)
+          if (HasHardInternalUses && !HasSoftExternalUses)
             continue;
         }
 
         bool HighCost = Rewriter.isHighCostExpansion(ExitValue, L, Inst);
         Value *ExitVal = Rewriter.expandCodeFor(ExitValue, PN->getType(), Inst);
 
         LLVM_DEBUG(dbgs() << "INDVARS: RLEV: AfterLoopVal = " << *ExitVal
                           << '\n'

llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll

 
 for.end:                                          ; preds = %for.body
 ; CHECK: for.end:
 ; CHECK-NOT: mul i32 %m, 186
 ; CHECK:%add.lcssa = phi i32 [ %add, %for.body ]
 ; CHECK-NEXT: ret i32 %add.lcssa
   ret i32 %add
 }
+
+; CHECK-LABEL: @test3(
+define void @test3(i32 %m) nounwind uwtable {
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %i.06 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %a.05 = phi i32 [ 0, %entry ], [ %add, %for.body ]
+  %add = add i32 %a.05, %m
+  mul i32 %add, 1
+  mul i32 %add, 1
+  mul i32 %add, 1
+  mul i32 %add, 1
+  mul i32 %add, 1
+  mul i32 %add, 1
+; CHECK: tail call void @func(i32 %add)
+  tail call void @func(i32 %add)
+  %inc = add nsw i32 %i.06, 1
+  %exitcond = icmp eq i32 %inc, 186
+  br i1 %exitcond, label %for.end, label %for.body
+
+for.end:                                          ; preds = %for.body
+; CHECK: for.end:
+; CHECK-NOT: mul i32 %m, 186
+; CHECK:%add.lcssa = phi i32 [ %add, %for.body ]
+; CHECK-NEXT: tail call void @func(i32 %add.lcssa)
+  tail call void @func(i32 %add)
+  ret void
+}