Index: include/llvm/Transforms/Scalar/Reassociate.h =================================================================== --- include/llvm/Transforms/Scalar/Reassociate.h +++ include/llvm/Transforms/Scalar/Reassociate.h @@ -82,7 +82,14 @@ static const unsigned GlobalReassociateLimit = 10; static const unsigned NumBinaryOps = Instruction::BinaryOpsEnd - Instruction::BinaryOpsBegin; - DenseMap, unsigned> PairMap[NumBinaryOps]; + + struct PairMapValue { + WeakVH Value1; + WeakVH Value2; + unsigned Score; + bool isValid() const { return Value1 && Value2; } + }; + DenseMap, PairMapValue> PairMap[NumBinaryOps]; bool MadeChange; Index: lib/Transforms/Scalar/Reassociate.cpp =================================================================== --- lib/Transforms/Scalar/Reassociate.cpp +++ lib/Transforms/Scalar/Reassociate.cpp @@ -2217,8 +2217,15 @@ if (std::less()(Op1, Op0)) std::swap(Op0, Op1); auto it = PairMap[Idx].find({Op0, Op1}); - if (it != PairMap[Idx].end()) - Score += it->second; + if (it != PairMap[Idx].end()) { + // Functions like BreakUpSubtract() can erase the Values we're using + // as keys and create new Values after we built the PairMap. There's a + // small chance that the new nodes can have the same address as + // something already in the table. We shouldn't accumulate the stored + // score in that case as it refers to the wrong Value. + if (it->second.isValid()) + Score += it->second.Score; + } unsigned MaxRank = std::max(Ops[i].Rank, Ops[j].Rank); if (Score > Max || (Score == Max && MaxRank < BestRank)) { @@ -2287,9 +2294,15 @@ std::swap(Op0, Op1); if (!Visited.insert({Op0, Op1}).second) continue; - auto res = PairMap[BinaryIdx].insert({{Op0, Op1}, 1}); - if (!res.second) - ++res.first->second; + auto res = PairMap[BinaryIdx].insert({{Op0, Op1}, {Op0, Op1, 1}}); + if (!res.second) { + // If either key value has been erased then we've got the same + // address by coincidence. That can't happen here because nothing is + // erasing values but it can happen by the time we're querying the + // map. + assert(res.first->second.isValid() && "WeakVH invalidated"); + ++res.first->second.Score; + } } } }