[PATCH] D51770: [IndVars] Set Changed when we delete dead instructions. PR38855

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked an inline comment as done.
mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2494
+            dyn_cast_or_null<Instruction>(DeadInsts.pop_back_val())) {
+      Changed = true;
       RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);
----------------
skatkov wrote:
> Changed |= ...?
> RecursivelyDeleteTriviallyDeadInstructions may return false indicating that it did not remove anything...
Not sure if it's happening in any real case (i.e. considering how DeadInsts is formed), but OK.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2502
   // loop may be sunk below the loop to reduce register pressure.
   sinkUnusedInvariants(L);
 
----------------
skatkov wrote:
> should it be fixed as well? And other places?
Maybe. It needs comprehensive analysis. I need to revisit any single place separately, let's not do it in this patch. It is to address a particular crash, however I agree that we might take a closer look into how `Changed` is sad.



https://reviews.llvm.org/D51770