[llvm] r320607 - [GVNHoist] Fix: PR35222 gvn-hoist incorrectly erases load

[Aditya Kumar via llvm-commits]

Author: hiraditya
Date: Wed Dec 13 11:40:07 2017
New Revision: 320607

URL: http://llvm.org/viewvc/llvm-project?rev=320607&view=rev
Log:
[GVNHoist] Fix: PR35222 gvn-hoist incorrectly erases load

w.r.t. the paper
"A Practical Improvement to the Partial Redundancy Elimination in SSA Form"
(https://sites.google.com/site/jongsoopark/home/ssapre.pdf)

Proper dominance check was missing here, so having a loopinfo should not be required.
Committing this diff as this fixes the bug, if there are
further concerns, I'll be happy to work on them.

Differential Revision: https://reviews.llvm.org/D39781

Added:
    llvm/trunk/test/Transforms/GVNHoist/pr35222-hoist-load.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=320607&r1=320606&r2=320607&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Wed Dec 13 11:40:07 2017
@@ -795,8 +795,8 @@ private:
       for (auto IDFB : IDFBlocks) { // TODO: Prune out useless CHI insertions.
         for (unsigned i = 0; i < V.size(); ++i) {
           CHIArg C = {VN, nullptr, nullptr};
-          if (DT->dominates(IDFB, V[i]->getParent())) { // Ignore spurious PDFs.
-            // InValue[V[i]->getParent()].push_back(std::make_pair(VN, V[i]));
+           // Ignore spurious PDFs.
+          if (DT->properlyDominates(IDFB, V[i]->getParent())) {
             OutValue[IDFB].push_back(C);
             DEBUG(dbgs() << "\nInsertion a CHI for BB: " << IDFB->getName()
                          << ", for Insn: " << *V[i]);

Added: llvm/trunk/test/Transforms/GVNHoist/pr35222-hoist-load.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVNHoist/pr35222-hoist-load.ll?rev=320607&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVNHoist/pr35222-hoist-load.ll (added)
+++ llvm/trunk/test/Transforms/GVNHoist/pr35222-hoist-load.ll Wed Dec 13 11:40:07 2017
@@ -0,0 +1,25 @@
+; RUN: opt -S -gvn-hoist < %s | FileCheck %s
+; CHECK: load
+; CHECK: load
+; Check that the load is not hoisted because the call can potentially
+; modify the global
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+ at heap = external global i32, align 4
+
+define i32 @build_tree() unnamed_addr {
+entry:
+  br label %do.body
+
+do.body:                                          ; preds = %do.body, %entry
+  %tmp9 = load i32, i32* @heap, align 4
+  %cmp = call i1 @pqdownheap(i32 %tmp9)
+  br i1 %cmp, label %do.body, label %do.end
+
+do.end:                                           ; preds = %do.body
+  %tmp20 = load i32, i32* @heap, align 4
+  ret i32 %tmp20
+}
+
+declare i1 @pqdownheap(i32)