[PATCH] D24517: GVN-hoist: fix store past load dependence analysis (PR30216)

[Daniel Berlin via llvm-commits]

dberlin accepted this revision.
dberlin added a comment.
This revision is now accepted and ready to land.

Fix these, otherwise looks fine


================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:347
@@ +346,3 @@
+      // Do not check whether MU aliases Def when MU occurs after OldPt.
+      if (BB == OldBB && firstInBB(OldPt, MU->getMemoryInst()))
+        continue;
----------------
Rather than repeatedly call firstInBB, can't you just set a boolean when you hit OldPt?


================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:351
@@ +350,3 @@
+      // Do not check whether MU aliases Def when MU occurs before NewPt.
+      if (BB == NewBB && firstInBB(MU->getMemoryInst(), NewPt))
+        continue;
----------------
Ditto. If it's the same BB, the before relationship is entirely controlled by whether you have hit NewPt or not.


https://reviews.llvm.org/D24517