[NFC][DSE]Change 'do-while' to 'for' loop to simplify code structure

[ebrevnov]

Summary

With 'for' loop there is is a single place where 'Current' is adjusted. It helps to avoid copy paste and makes a bit easy to understand overall loop controll flow.

[ebrevnov]

ping @fhahn

[fhahn]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1391–1392

The braces here and at the other changed lines are now redundant. Would be good to remove them.

1450–1451

perhaps it would be simpler to just have the break as last statement of the loop?

[ebrevnov]

Fixed as requested

[ebrevnov]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1450–1451

Good suggestion. Done.

[fhahn]

LGTM, this looks like a nice simplification to ensure Current is updated correctly at the end of each iteration. Thanks!

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

getDomMemoryDef(MemoryDef *KillingDef, MemoryAccess *StartAccess,
                   bool IsMemTerm, unsigned &PartialLimit) {
     if (ScanLimit == 0 || WalkerStepLimit == 0) {
       LLVM_DEBUG(dbgs() << "\n    ...  hit scan limit\n");
       return None;
     }
 
     MemoryAccess *Current = StartAccess;
     Instruction *KillingI = KillingDef->getMemoryInst();
-    bool StepAgain;
     LLVM_DEBUG(dbgs() << "  trying to get dominating access\n");
 
     // Find the next clobbering Mod access for DefLoc, starting at StartAccess.
     Optional<MemoryLocation> CurrentLoc;
-    do {
-      StepAgain = false;
+    for (;; Current = cast<MemoryDef>(Current)->getDefiningAccess()) {
       LLVM_DEBUG({
         dbgs() << "   visiting " << *Current;
         if (!MSSA.isLiveOnEntryDef(Current) && isa<MemoryUseOrDef>(Current))
           dbgs() << " (" << *cast<MemoryUseOrDef>(Current)->getMemoryInst()
                  << ")";
         dbgs() << "\n";
       });
 
for (;; Current = cast<MemoryDef>(Current)->getDefiningAccess()) {
         LLVM_DEBUG(dbgs() << "   ...  found MemoryPhi\n");
         return Current;
       }
 
       // Below, check if CurrentDef is a valid candidate to be eliminated by
       // KillingDef. If it is not, check the next candidate.
       MemoryDef *CurrentDef = cast<MemoryDef>(Current);
       Instruction *CurrentI = CurrentDef->getMemoryInst();
 
-      if (canSkipDef(CurrentDef, !isInvisibleToCallerBeforeRet(DefUO))) {
-        StepAgain = true;
-        Current = CurrentDef->getDefiningAccess();
+      if (canSkipDef(CurrentDef, !isInvisibleToCallerBeforeRet(DefUO)))
         continue;
-      }
 
       // Before we try to remove anything, check for any extra throwing
       // instructions that block us from DSEing
       if (mayThrowBetween(KillingI, CurrentI, DefUO)) {
         LLVM_DEBUG(dbgs() << "  ... skip, may throw!\n");
         return None;
       }
 
for (;; Current = cast<MemoryDef>(Current)->getDefiningAccess()) {
             return false;
           })) {
         LLVM_DEBUG(dbgs() << "   ...  found a read clobber\n");
         return None;
       }
 
       // If Current cannot be analyzed or is not removable, check the next
       // candidate.
-      if (!hasAnalyzableMemoryWrite(CurrentI, TLI) || !isRemovable(CurrentI)) {
-        StepAgain = true;
-        Current = CurrentDef->getDefiningAccess();
+      if (!hasAnalyzableMemoryWrite(CurrentI, TLI) || !isRemovable(CurrentI))
         continue;
-      }
 
       // If Current does not have an analyzable write location, skip it
       CurrentLoc = getLocForWriteEx(CurrentI);
-      if (!CurrentLoc) {
-        StepAgain = true;
-        Current = CurrentDef->getDefiningAccess();
+      if (!CurrentLoc)
         continue;
-      }
 
       // AliasAnalysis does not account for loops. Limit elimination to
       // candidates for which we can guarantee they always store to the same
       // memory location and not located in different loops.
       if (!isGuaranteedLoopIndependent(CurrentI, KillingI, *CurrentLoc)) {
         LLVM_DEBUG(dbgs() << "  ... not guaranteed loop independent\n");
-        StepAgain = true;
-        Current = CurrentDef->getDefiningAccess();
         WalkerStepLimit -= 1;
         continue;
       }
 
       if (IsMemTerm) {
         // If the killing def is a memory terminator (e.g. lifetime.end), check
         // the next candidate if the current Current does not write the same
         // underlying object as the terminator.
-        if (!isMemTerminator(*CurrentLoc, CurrentI, KillingI)) {
-          StepAgain = true;
-          Current = CurrentDef->getDefiningAccess();
-        }
+        if (!isMemTerminator(*CurrentLoc, CurrentI, KillingI))
           continue;
       } else {
         int64_t InstWriteOffset, DepWriteOffset;
         auto OR = isOverwrite(KillingI, CurrentI, DefLoc, *CurrentLoc,
                               DepWriteOffset, InstWriteOffset);
         // If Current does not write to the same object as KillingDef, check
         // the next candidate.
-        if (OR == OW_Unknown) {
-          StepAgain = true;
-          Current = CurrentDef->getDefiningAccess();
-        } else if (OR == OW_MaybePartial) {
+        if (OR == OW_Unknown)
+          continue;
+        else if (OR == OW_MaybePartial) {
           // If KillingDef only partially overwrites Current, check the next
           // candidate if the partial step limit is exceeded. This aggressively
           // limits the number of candidates for partial store elimination,
           // which are less likely to be removable in the end.
           if (PartialLimit <= 1) {
-            StepAgain = true;
-            Current = CurrentDef->getDefiningAccess();
             WalkerStepLimit -= 1;
             continue;
           }
           PartialLimit -= 1;
         }
       }
-    } while (StepAgain);
+      break;
+    };
 
     // Accesses to objects accessible after the function returns can only be
     // eliminated if the access is killed along all paths to the exit. Collect
     // the blocks with killing (=completely overwriting MemoryDefs) and check if
     // they cover all paths from EarlierAccess to any function exit.
     SmallPtrSet<Instruction *, 16> KillingDefs;
     KillingDefs.insert(KillingDef->getMemoryInst());
     MemoryAccess *EarlierAccess = Current;