<html>
    <head>
      <base href="https://llvm.org/bugs/" />
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW --- - NewGVN doesn't merge unconditional branches."
   href="https://llvm.org/bugs/show_bug.cgi?id=31468">31468</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>NewGVN doesn't merge unconditional branches.
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>libraries
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>trunk
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>normal
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>Scalar Optimizations
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>davide@freebsd.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>davide@freebsd.org
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>dberlin@dberlin.org, llvm-bugs@lists.llvm.org
          </td>
        </tr>

        <tr>
          <th>Blocks</th>
          <td>30995
          </td>
        </tr>

        <tr>
          <th>Classification</th>
          <td>Unclassified
          </td>
        </tr></table>
      <p>
        <div>
        <pre>The code living in GVN.cpp claims this is done to expose "more PRE
opportunities". Fair enough, but I'm not entirely sure this is relevant anymore
in NewGVN as we're writing a real PRE infrastructure. 

Fixing is relatively trivial (patch inline), but not sure if really needed.
Dan, what you think? If we don't want to change this code, I'll modify the
failing test and call it a day (basic.ll)

diff --git a/lib/Transforms/Scalar/NewGVN.cpp
b/lib/Transforms/Scalar/NewGVN.cpp
index 1f12d10..430385a 100644
--- a/lib/Transforms/Scalar/NewGVN.cpp
+++ b/lib/Transforms/Scalar/NewGVN.cpp
@@ -1378,6 +1378,20 @@ bool NewGVN::runGVN(Function &F, DominatorTree *_DT,
AssumptionCache *_AC,
   unsigned ICount = 0;
   SmallPtrSet<BasicBlock *, 16> VisitedBlocks;

+  // Do a sweep over all the basic blocks to merge unconditional branches.
+  for (Function::iterator FI = F.begin(), FE = F.end(); FI != FE;) {
+    BasicBlock *BB = &*FI++;
+
+    // In theory here we could pass MemSSA if/when MergeBlockIntoPredecessor
+    // will grow version which accepts the analysis.
+    bool removedBlock = MergeBlockIntoPredecessor(
+        BB, DT, nullptr /* LoopInfo */, nullptr /* MemDep */);
+
+    if (removedBlock)
+      NumGVNBlocksDeleted++;
+    Changed |= removedBlock;
+  }
+
   // Note: We want RPO traversal of the blocks, which is not quite the same as
   // dominator tree order, particularly with regard whether backedges get
   // visited first or second, given a block with multiple successors.
diff --git a/test/Transforms/NewGVN/basic.ll b/test/Transforms/NewGVN/basic.ll
index f6670aa..f68823a 100644
--- a/test/Transforms/NewGVN/basic.ll
+++ b/test/Transforms/NewGVN/basic.ll
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; RUN: opt < %s -newgvn -S | FileCheck %s

 define i32 @main() {</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>