[PATCH] D37460: [GVN] Prevent LoadPRE from hoisting across instructions that don't pass control flow to successors

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: test/Transforms/GVN/PRE/local-pre.ll:48
+  call void @may_exit() nounwind
+  %b = sdiv i32 %p, %q
+  ret i32 %b
----------------
efriedma wrote:
> GVN got smarter, so this isn't actually testing what it's supposed to test anymore.  Maybe try replacing "icmp eq i32 %p, %q" with "icmp eq i32 %p, %r", where "%r" is a parameter.  And I guess pass %a to may_exit(), so it won't get moved if we come up with some new form of sinking in the future.
What is the difference between `icmp eq i32 %p, %q` where `%q` is a parameter and `icmp eq i32 %p, %r` where `%r` is a parameter? If you mean that we should branch by unknown condition then we could just add parameter `i5 %cond`.

Anyways, I don't think that extension of the test set can somehow block us from merging this functional fix. I also don't see what is the conceptual difference between this test and what you describe. The transformation across `may_exit` still doesn't happen. Do you mind if we consider adding new tests as NFC later?


https://reviews.llvm.org/D37460