[PATCH] D98602: [SCCP] Avoid modifying AdditionalUsers while iterating over it

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 14 10:15:35 PDT 2021


lebedev.ri added a comment.

Test missing.

Am i understanding correctly that the problem is *only* that the inner `SmallPtrSet` may get new entries added to it?
I think the fix depends on the expected behavior here, in particular, two things stand out:

- when looping over that set there, if a new unique entry is added in process, should it be ignored, or handled?
- what if said new entry isn't unique? should it be re-handled?

I'm not sure about the latter, but not handling the former //seems// like a bug.
I think we should instead change `AdditionalUsers` from `DenseMap<Value *, SmallPtrSet<User *, 2>>` to `DenseMap<Value *, SetVector<User *, 2>>`,
and change the loop to

  // NOTE: new entries may be added to the vector in-process, invalidating iterators.
  for(int I = 0; I != Iter->second.size(); ++I) {
    User *U = Iter->second[I];
    <...>
  }

But it is also possible that i don't know what i'm talking about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98602/new/

https://reviews.llvm.org/D98602



More information about the llvm-commits mailing list