[PATCH] D125205: Replace the custom linked list in LeaderTableEntry with TinyPtrVector.

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 10 01:41:27 PDT 2022


nikic added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Scalar/GVN.h:284
+        auto eraseBI = BI--;
+        entry.BB.erase(eraseBI);
       }
----------------
This looks incorrect in multiple ways. If we're at the first instruction, this will step before the begin iterator. And the end iterator is invalidated in either case. I think you need something like this:
```
VI = entry.Val.erase(VI);
BI = entry.BB.erase(BI);
VE = entry.Val.end();
continue;
```


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:2113
+    }
+  }
+
----------------
Can be just `return is_contained(entry.BB, BB)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125205



More information about the llvm-commits mailing list