[PATCH] D141712: [GVN] Improve PRE on load instructions

Thu Jan 26 03:14:05 PST 2023

nikic added a comment.

It looks like the new version of the patch ended up being more expensive in terms of compile-time: Original <http://llvm-compile-time-tracker.com/compare.php?from=fe2ca62e92c82efcbbd10916e352bdfeddb80e19&to=1d81778eee4d4c38db40ec50a5e18ac76e251b44&stat=instructions:u>, New <http://llvm-compile-time-tracker.com/compare.php?from=287508cd9c4396c8845d92310d258879202a179e&to=5f1448fe1585b5677d5f0064e4eeac3b493d8a18&stat=instructions%3Au>

I wonder whether this is because not removing the load essentially always forces an extra GVN iteration?

================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:1462
+        if (uint32_t ValNo = VN.lookup(OldLoad, false))
+          removeFromLeaderTable(ValNo, OldLoad, OldLoad->getParent());
+        // To avoid deleting an instruction from different BB, we just leave
----------------
If we're not removing the load, we probably shouldn't be removing it from the leader table either?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141712