[PATCH] D33056: [NewGVN] When a store becomes the leader, update the memory leader for the class

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

I think we might just be missing a check for store count (as it's done in the other place we call `getNextMemoryLeader()`).
Please let me know what you think, and thanks for catching the mistake!

  diff --git a/lib/Transforms/Scalar/NewGVN.cpp b/lib/Transforms/Scalar/NewGVN.cpp
  index 3c9850b..f7182f2 100644
  --- a/lib/Transforms/Scalar/NewGVN.cpp
  +++ b/lib/Transforms/Scalar/NewGVN.cpp
  @@ -1381,7 +1381,7 @@ bool NewGVN::setMemoryClass(const MemoryAccess *From,
           NewClass->memory_insert(MP);
           // This may have killed the class if it had no non-memory members
           if (OldClass->getMemoryLeader() == From) {
  -          if (OldClass->memory_empty()) {
  +          if (OldClass->memory_empty() && OldClass->getStoreCount() == 0) {
               OldClass->setMemoryLeader(nullptr);
             } else {
               OldClass->setMemoryLeader(getNextMemoryLeader(OldClass));
  
  fluttershy at SONY-PC C:\Users\fluttershy\work\llvm


https://reviews.llvm.org/D33056