[SCCP] Correct the made changes behavior

[bcl5980]

Summary

For now in some code path, even if it can be removed we still set MadeChanges to true. This patch fix them to make sure the MadeChanges set to true only if it is really changed.

[chill]

llvm/lib/Transforms/Utils/SCCPSolver.cpp

238 ↗

(On Diff #500652)

tryToReplaceWithConstant will return true if it made a change and we should set the flag regardless of whether we can remove the instruction.

[nikic]

llvm/lib/Transforms/IPO/SCCP.cpp

215

Even in this case we're going to do a changeToUnreachable() below, so it's still changed?

[bcl5980]

llvm/lib/Transforms/IPO/SCCP.cpp

215

Oh thanks for the mention. You are right, generally it will change first non-phi instruction to unreachable.
How about if the entry block is already unreachable? It will replace unreachable to unreachable again. It looks this case we needn't set MadeChanges to true.
I have some local code to run ipsccp until no changed and dead loop happen here. This is the motivation of the patch. And I think I need to find some other way to workaround it maybe.

llvm/lib/Transforms/IPO/SCCP.cpp

for (Function &F : M) {
     }
 
     SmallPtrSet<Value *, 32> InsertedValues;
     for (BasicBlock &BB : F) {
       if (!Solver.isBlockExecutable(&BB)) {
         LLVM_DEBUG(dbgs() << "  BasicBlock Dead:" << BB);
         ++NumDeadBlocks;
 
+        if (&BB != &F.front()) {
           MadeChanges = true;
-
-        if (&BB != &F.front())
           BlocksToErase.push_back(&BB);
+        }
         continue;
       }
 
       MadeChanges |= Solver.simplifyInstsInBlock(
           BB, InsertedValues, NumInstRemoved, NumInstReplaced);
     }
 
     DomTreeUpdater DTU = IsFuncSpecEnabled && Specializer.isClonedFunction(&F)