Index: lib/Transforms/Scalar/NaryReassociate.cpp =================================================================== --- lib/Transforms/Scalar/NaryReassociate.cpp +++ lib/Transforms/Scalar/NaryReassociate.cpp @@ -188,7 +188,7 @@ // foo(a + b); // if (p2) // bar(a + b); - DenseMap> SeenExprs; + DenseMap> SeenExprs; }; } // anonymous namespace @@ -252,13 +252,15 @@ Changed = true; SE->forgetValue(I); I->replaceAllUsesWith(NewI); + // If SeenExprs constains I's WeakVH, that entry will be replaced with + // nullptr. RecursivelyDeleteTriviallyDeadInstructions(I, TLI); I = NewI; } // Add the rewritten instruction to SeenExprs; the original instruction // is deleted. const SCEV *NewSCEV = SE->getSCEV(I); - SeenExprs[NewSCEV].push_back(I); + SeenExprs[NewSCEV].push_back(WeakVH(I)); // Ideally, NewSCEV should equal OldSCEV because tryReassociate(I) // is equivalent to I. However, ScalarEvolution::getSCEV may // weaken nsw causing NewSCEV not to equal OldSCEV. For example, suppose @@ -278,7 +280,7 @@ // // This improvement is exercised in @reassociate_gep_nsw in nary-gep.ll. if (NewSCEV != OldSCEV) - SeenExprs[OldSCEV].push_back(I); + SeenExprs[OldSCEV].push_back(WeakVH(I)); } } } @@ -562,9 +564,13 @@ // future instruction either. Therefore, we pop it out of the stack. This // optimization makes the algorithm O(n). while (!Candidates.empty()) { - Instruction *Candidate = Candidates.back(); - if (DT->dominates(Candidate, Dominatee)) - return Candidate; + // Candidates stores WeakVHs, so a candidate can be nullptr if it's removed + // during rewriting. + if (Value *Candidate = Candidates.back()) { + Instruction *CandidateInstruction = cast(Candidate); + if (DT->dominates(CandidateInstruction, Dominatee)) + return CandidateInstruction; + } Candidates.pop_back(); } return nullptr; Index: test/Transforms/NaryReassociate/pr24301.ll =================================================================== --- /dev/null +++ test/Transforms/NaryReassociate/pr24301.ll @@ -0,0 +1,14 @@ +; RUN: opt < %s -nary-reassociate -S | FileCheck %s + +define i32 @foo(i32 %tmp4) { +; CHECK-LABEL: @foo( +entry: + %tmp5 = add i32 %tmp4, 8 + %tmp13 = add i32 %tmp4, -128 ; deleted + %tmp14 = add i32 %tmp13, 8 ; => %tmp5 + -128 + %tmp21 = add i32 119, %tmp4 + ; do not rewrite %tmp23 against %tmp13 because %tmp13 is already deleted + %tmp23 = add i32 %tmp21, -128 +; CHECK: %tmp23 = add i32 %tmp21, -128 + ret i32 %tmp23 +}