[InstCombine] Rename worklist methods; NFC

This renames Worklist.AddDeferred() to Worklist.add() and
Worklist.Add() to Worklist.push(). The intention here is that
Worklist.add() should be the go-to method for explicit worklist
management, while the raw Worklist.push() is mostly for
InstCombine internals. I will then migrate uses of Worklist.push()
to Worklist.add() in followup changes.

As suggested by spatel on D73411 I'm also changing the remaining
method names to lowercase first character, in line with current
coding standards.

Differential Revision: https://reviews.llvm.org/D73745

llvm/include/llvm/Transforms/InstCombine/InstCombineWorklist.h

@@ -40,9 +40,24 @@ class InstCombineWorklist {
 
   bool isEmpty() const { return Worklist.empty(); }
 
-  /// Add - Add the specified instruction to the worklist if it isn't already
-  /// in it.
-  void Add(Instruction *I) {
+  /// Add instruction to the worklist.
+  /// Instructions will be visited in the order they are added.
+  /// You likely want to use this method.
+  void add(Instruction *I) {
+    if (Deferred.insert(I))
+      LLVM_DEBUG(dbgs() << "IC: ADD DEFERRED: " << *I << '\n');
+  }
+
+  /// Add value to the worklist if it is an instruction.
+  /// Instructions will be visited in the order they are added.
+  void addValue(Value *V) {
+    if (Instruction *I = dyn_cast<Instruction>(V))
+      add(I);
+  }
+
+  /// Push the instruction onto the worklist stack.
+  /// Instructions that have been added first will be visited last.
+  void push(Instruction *I) {
     assert(I);
     assert(I->getParent() && "Instruction not inserted yet?");
 
@@ -52,26 +67,21 @@ class InstCombineWorklist {
     }
   }
 
-  void AddValue(Value *V) {
+  void pushValue(Value *V) {
     if (Instruction *I = dyn_cast<Instruction>(V))
-      Add(I);
-  }
-
-  void AddDeferred(Instruction *I) {
-    if (Deferred.insert(I))
-      LLVM_DEBUG(dbgs() << "IC: ADD DEFERRED: " << *I << '\n');
+      push(I);
   }
 
-  void AddDeferredInstructions() {
+  void addDeferredInstructions() {
     for (Instruction *I : reverse(Deferred))
-      Add(I);
+      push(I);
     Deferred.clear();
   }
 
   /// AddInitialGroup - Add the specified batch of stuff in reverse order.
   /// which should only be done when the worklist is empty and when the group
   /// has no duplicates.
-  void AddInitialGroup(ArrayRef<Instruction *> List) {
+  void addInitialGroup(ArrayRef<Instruction *> List) {
     assert(Worklist.empty() && "Worklist must be empty to add initial group");
     Worklist.reserve(List.size()+16);
     WorklistMap.reserve(List.size());
@@ -84,8 +94,8 @@ class InstCombineWorklist {
     }
   }
 
-  // Remove - remove I from the worklist if it exists.
-  void Remove(Instruction *I) {
+  /// Remove I from the worklist if it exists.
+  void remove(Instruction *I) {
     DenseMap<Instruction*, unsigned>::iterator It = WorklistMap.find(I);
     if (It == WorklistMap.end()) return; // Not in worklist.
 
@@ -96,25 +106,22 @@ class InstCombineWorklist {
     Deferred.remove(I);
   }
 
-  Instruction *RemoveOne() {
+  Instruction *removeOne() {
     Instruction *I = Worklist.pop_back_val();
     WorklistMap.erase(I);
     return I;
   }
 
-  /// AddUsersToWorkList - When an instruction is simplified, add all users of
-  /// the instruction to the work lists because they might get more simplified
-  /// now.
-  ///
-  void AddUsersToWorkList(Instruction &I) {
+  /// When an instruction is simplified, add all users of the instruction
+  /// to the work lists because they might get more simplified now.
+  void pushUsersToWorkList(Instruction &I) {
     for (User *U : I.users())
-      Add(cast<Instruction>(U));
+      push(cast<Instruction>(U));
   }
 
 
-  /// Zap - check that the worklist is empty and nuke the backing store for
-  /// the map if it is large.
-  void Zap() {
+  /// Check that the worklist is empty and nuke the backing store for the map.
+  void zap() {
     assert(WorklistMap.empty() && "Worklist empty, but map not?");
     assert(Deferred.empty() && "Deferred instructions left over");
 

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

@@ -2015,7 +2015,7 @@ Instruction *InstCombiner::matchBSwap(BinaryOperator &Or) {
   LastInst->removeFromParent();
 
   for (auto *Inst : Insts)
-    Worklist.Add(Inst);
+    Worklist.push(Inst);
   return LastInst;
 }
 
@@ -3109,7 +3109,7 @@ Instruction *InstCombiner::visitXor(BinaryOperator &I) {
       if (match(Op0, m_Or(m_Value(X), m_APInt(C))) &&
           MaskedValueIsZero(X, *C, 0, &I)) {
         Constant *NewC = ConstantInt::get(I.getType(), *C ^ *RHSC);
-        Worklist.Add(cast<Instruction>(Op0));
+        Worklist.push(cast<Instruction>(Op0));
         I.setOperand(0, X);
         I.setOperand(1, NewC);
         return &I;

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

@@ -4818,7 +4818,7 @@ bool InstCombiner::transformConstExprCastCall(CallBase &Call) {
         // Otherwise, it's a call, just insert cast right after the call.
         InsertNewInstBefore(NC, *Caller);
       }
-      Worklist.AddUsersToWorkList(*Caller);
+      Worklist.pushUsersToWorkList(*Caller);
     } else {
       NV = UndefValue::get(Caller->getType());
     }

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

@@ -1821,7 +1821,7 @@ Instruction *InstCombiner::commonPointerCastTransforms(CastInst &CI) {
       // Changing the cast operand is usually not a good idea but it is safe
       // here because the pointer operand is being replaced with another
       // pointer operand so the opcode doesn't need to change.
-      Worklist.Add(GEP);
+      Worklist.push(GEP);
       CI.setOperand(0, GEP->getOperand(0));
       return &CI;
     }
@@ -2363,7 +2363,7 @@ Instruction *InstCombiner::optimizeBitCastFromPhi(CastInst &CI, PHINode *PN) {
         auto *NewBC =
           cast<BitCastInst>(Builder.CreateBitCast(NewPN, SrcTy));
         SI->setOperand(0, NewBC);
-        Worklist.Add(SI);
+        Worklist.push(SI);
         assert(hasStoreUsersOnly(*NewBC));
       }
       else if (auto *BCI = dyn_cast<BitCastInst>(V)) {

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

@@ -1577,7 +1577,7 @@ Instruction *InstCombiner::foldICmpXorConstant(ICmpInst &Cmp,
     // the operation, just stop using the Xor.
     if (!XorC->isNegative()) {
       Cmp.setOperand(0, X);
-      Worklist.Add(Xor);
+      Worklist.push(Xor);
       return &Cmp;
     }
 
@@ -1687,7 +1687,7 @@ Instruction *InstCombiner::foldICmpAndShift(ICmpInst &Cmp, BinaryOperator *And,
         APInt NewAndCst = IsShl ? C2.lshr(*C3) : C2.shl(*C3);
         And->setOperand(1, ConstantInt::get(And->getType(), NewAndCst));
         And->setOperand(0, Shift->getOperand(0));
-        Worklist.Add(Shift); // Shift is dead.
+        Worklist.push(Shift); // Shift is dead.
         return &Cmp;
       }
     }
@@ -4658,7 +4658,7 @@ static Instruction *processUMulZExtIdiom(ICmpInst &I, Value *MulVal,
   Function *F = Intrinsic::getDeclaration(
       I.getModule(), Intrinsic::umul_with_overflow, MulType);
   CallInst *Call = Builder.CreateCall(F, {MulA, MulB}, "umul");
-  IC.Worklist.Add(MulInstr);
+  IC.Worklist.push(MulInstr);
 
   // If there are uses of mul result other than the comparison, we know that
   // they are truncation or binary AND. Change them to use result of
@@ -4685,11 +4685,11 @@ static Instruction *processUMulZExtIdiom(ICmpInst &I, Value *MulVal,
       } else {
         llvm_unreachable("Unexpected Binary operation");
       }
-      IC.Worklist.Add(cast<Instruction>(U));
+      IC.Worklist.push(cast<Instruction>(U));
     }
   }
   if (isa<Instruction>(OtherVal))
-    IC.Worklist.Add(cast<Instruction>(OtherVal));
+    IC.Worklist.push(cast<Instruction>(OtherVal));
 
   // The original icmp gets replaced with the overflow value, maybe inverted
   // depending on predicate.

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

@@ -648,7 +648,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner
            "New instruction already inserted into a basic block!");
     BasicBlock *BB = Old.getParent();
     BB->getInstList().insert(Old.getIterator(), New); // Insert inst
-    Worklist.Add(New);
+    Worklist.push(New);
     return New;
   }
 
@@ -669,7 +669,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner
     // no changes were made to the program.
     if (I.use_empty()) return nullptr;
 
-    Worklist.AddUsersToWorkList(I); // Add all modified instrs to worklist.
+    Worklist.pushUsersToWorkList(I); // Add all modified instrs to worklist.
 
     // If we are replacing the instruction with itself, this must be in a
     // segment of unreachable code, so just clobber the instruction.
@@ -718,9 +718,9 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner
     if (I.getNumOperands() < 8) {
       for (Use &Operand : I.operands())
         if (auto *Inst = dyn_cast<Instruction>(Operand))
-          Worklist.Add(Inst);
+          Worklist.push(Inst);
     }
-    Worklist.Remove(&I);
+    Worklist.remove(&I);
     I.eraseFromParent();
     MadeIRChange = true;
     return nullptr; // Don't do anything with FI

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

@@ -973,7 +973,7 @@ Instruction *InstCombiner::visitLoadInst(LoadInst &LI) {
 
   // Replace GEP indices if possible.
   if (Instruction *NewGEPI = replaceGEPIdxWithZero(*this, Op, LI)) {
-      Worklist.Add(NewGEPI);
+      Worklist.push(NewGEPI);
       return &LI;
   }
 
@@ -1384,7 +1384,7 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) {
 
   // Replace GEP indices if possible.
   if (Instruction *NewGEPI = replaceGEPIdxWithZero(*this, Ptr, SI)) {
-      Worklist.Add(NewGEPI);
+      Worklist.push(NewGEPI);
       return &SI;
   }
 
@@ -1434,7 +1434,7 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) {
         // Manually add back the original store to the worklist now, so it will
         // be processed after the operands of the removed store, as this may
         // expose additional DSE opportunities.
-        Worklist.Add(&SI);
+        Worklist.push(&SI);
         eraseInstFromFunction(*PrevSI);
         return nullptr;
       }
@@ -1466,7 +1466,7 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) {
     if (!isa<UndefValue>(Val)) {
       SI.setOperand(0, UndefValue::get(Val->getType()));
       if (Instruction *U = dyn_cast<Instruction>(Val))
-        Worklist.Add(U);  // Dropped a use.
+        Worklist.push(U);  // Dropped a use.
     }
     return nullptr;  // Do not modify these!
   }

llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

@@ -621,11 +621,11 @@ bool InstCombiner::simplifyDivRemOfSelectWithZeroOp(BinaryOperator &I) {
          I != E; ++I) {
       if (*I == SI) {
         *I = SI->getOperand(NonNullOperand);
-        Worklist.Add(&*BBI);
+        Worklist.push(&*BBI);
       } else if (*I == SelectCond) {
         *I = NonNullOperand == 1 ? ConstantInt::getTrue(CondTy)
                                  : ConstantInt::getFalse(CondTy);
-        Worklist.Add(&*BBI);
+        Worklist.push(&*BBI);
       }
     }
 
@@ -1418,7 +1418,7 @@ Instruction *InstCombiner::visitSRem(BinaryOperator &I) {
     const APInt *Y;
     // X % -Y -> X % Y
     if (match(Op1, m_Negative(Y)) && !Y->isMinSignedValue()) {
-      Worklist.AddValue(I.getOperand(1));
+      Worklist.pushValue(I.getOperand(1));
       I.setOperand(1, ConstantInt::get(I.getType(), -*Y));
       return &I;
     }
@@ -1469,7 +1469,7 @@ Instruction *InstCombiner::visitSRem(BinaryOperator &I) {
 
       Constant *NewRHSV = ConstantVector::get(Elts);
       if (NewRHSV != C) {  // Don't loop on -MININT
-        Worklist.AddValue(I.getOperand(1));
+        Worklist.pushValue(I.getOperand(1));
         I.setOperand(1, NewRHSV);
         return &I;
       }

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

@@ -2404,7 +2404,7 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
     SI.setOperand(1, FalseVal);
     SI.setOperand(2, TrueVal);
     SI.swapProfMetadata();
-    Worklist.Add(Cond);
+    Worklist.push(Cond);
     return &SI;
   }
 
@@ -2747,14 +2747,14 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
     if (auto *TrueBOSI = dyn_cast<SelectInst>(TrueBO->getOperand(0))) {
       if (TrueBOSI->getCondition() == CondVal) {
         TrueBO->setOperand(0, TrueBOSI->getTrueValue());
-        Worklist.Add(TrueBO);
+        Worklist.push(TrueBO);
         return &SI;
       }
     }
     if (auto *TrueBOSI = dyn_cast<SelectInst>(TrueBO->getOperand(1))) {
       if (TrueBOSI->getCondition() == CondVal) {
         TrueBO->setOperand(1, TrueBOSI->getTrueValue());
-        Worklist.Add(TrueBO);
+        Worklist.push(TrueBO);
         return &SI;
       }
     }
@@ -2767,14 +2767,14 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
     if (auto *FalseBOSI = dyn_cast<SelectInst>(FalseBO->getOperand(0))) {
       if (FalseBOSI->getCondition() == CondVal) {
         FalseBO->setOperand(0, FalseBOSI->getFalseValue());
-        Worklist.Add(FalseBO);
+        Worklist.push(FalseBO);
         return &SI;
       }
     }
     if (auto *FalseBOSI = dyn_cast<SelectInst>(FalseBO->getOperand(1))) {
       if (FalseBOSI->getCondition() == CondVal) {
         FalseBO->setOperand(1, FalseBOSI->getFalseValue());
-        Worklist.Add(FalseBO);
+        Worklist.push(FalseBO);
         return &SI;
       }
     }

llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp

@@ -605,7 +605,7 @@ static Value *getShiftedValue(Value *V, unsigned NumBits, bool isLeftShift,
   }
 
   Instruction *I = cast<Instruction>(V);
-  IC.Worklist.Add(I);
+  IC.Worklist.push(I);
 
   switch (I->getOpcode()) {
   default: llvm_unreachable("Inconsistency with CanEvaluateShifted");

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

@@ -1309,7 +1309,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
     // If this is inserting an element that isn't demanded, remove this
     // insertelement.
     if (IdxNo >= VWidth || !DemandedElts[IdxNo]) {
-      Worklist.Add(I);
+      Worklist.push(I);
       return I->getOperand(0);
     }
 
@@ -1590,7 +1590,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
       // use Arg0 if DemandedElts[0] is clear like we do for other intrinsics.
       // Instead we should return a zero vector.
       if (!DemandedElts[0]) {
-        Worklist.Add(II);
+        Worklist.push(II);
         return ConstantAggregateZero::get(II->getType());
       }
 
@@ -1609,7 +1609,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
 
       // If lowest element of a scalar op isn't used then use Arg0.
       if (!DemandedElts[0]) {
-        Worklist.Add(II);
+        Worklist.push(II);
         return II->getArgOperand(0);
       }
       // TODO: If only low elt lower SQRT to FSQRT (with rounding/exceptions
@@ -1629,7 +1629,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
 
       // If lowest element of a scalar op isn't used then use Arg0.
       if (!DemandedElts[0]) {
-        Worklist.Add(II);
+        Worklist.push(II);
         return II->getArgOperand(0);
       }
 
@@ -1656,7 +1656,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
 
       // If lowest element of a scalar op isn't used then use Arg0.
       if (!DemandedElts[0]) {
-        Worklist.Add(II);
+        Worklist.push(II);
         return II->getArgOperand(0);
       }
 
@@ -1690,7 +1690,7 @@ Value *InstCombiner::SimplifyDemandedVectorElts(Value *V, APInt DemandedElts,
 
       // If lowest element of a scalar op isn't used then use Arg0.
       if (!DemandedElts[0]) {
-        Worklist.Add(II);
+        Worklist.push(II);
         return II->getArgOperand(0);
       }