[NewGVN] Skip uses in unreachable blocks

[davide]

Summary

Fixes https://llvm.org/bugs/show_bug.cgi?id=31758
Otherwise we ask for a domtree node that's not there, and we crash.

[• dberlin]

lib/Transforms/Scalar/NewGVN.cpp

1946 ↗

(On Diff #85798)

Do you need this lookup?
Won't localnum already be 0, because we now zero the dfs number for uses in unreachable blocks?

[davide]

Landing this fix as it is correct (after clarifying/checking with Danny on IRC).

[davide]

For archeologists:

16:22 < davide> DannyB: I may be mistaken, but are we marking users in unreachable blocks with InstrDFSNum == 0 ? Currently we just set that if an instruction is trivially dead
16:23 < davide> DannyB: in the example, the load with SSA name %tmp136 has InstrDFSNum zero, but the gep in bb138 has a non-zero InstrDFSNum
16:24 < DannyB> oh interesting
16:24 < DannyB> how did thathappen?
16:24 < DannyB> oh
16:24 < DannyB> yeah
16:24 < DannyB> you are right
16:24 < DannyB> because they may become reachable later
16:24 < davide> yeah

llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp

for (auto &U : D->uses()) {
           IBlock = P->getIncomingBlock(U);
           // Make phi node users appear last in the incoming block
           // they are from.
           VD.LocalNum = InstrDFS.size() + 1;
         } else {
           IBlock = I->getParent();
           VD.LocalNum = InstrDFS.lookup(I);
         }
+
+        // Skip uses in unreachable blocks, as we're going
+        // to delete them.
+        if (ReachableBlocks.count(IBlock) == 0)
+          continue;
+
         DomTreeNode *DomNode = DT->getNode(IBlock);
         VD.DFSIn = DomNode->getDFSNumIn();
         VD.DFSOut = DomNode->getDFSNumOut();
         VD.U = &U;
         DFSOrderedSet.emplace_back(VD);
       }
     }
   }

llvm/trunk/test/Transforms/NewGVN/pr31758.ll

+; RUN: opt -newgvn %s -S -o - | FileCheck %s
+
+%struct.dipsy = type {}
+%struct.fluttershy = type { %struct.dipsy* }
+%struct.patatino = type {}
+
+define void @tinkywinky() {
+bb:
+  br label %bb90
+
+bb90:
+  %tmp = getelementptr inbounds %struct.fluttershy, %struct.fluttershy* undef, i64 0, i32 0
+  %tmp91 = bitcast %struct.dipsy** %tmp to %struct.patatino**
+  %tmp92 = load %struct.patatino*, %struct.patatino** %tmp91, align 8
+  %tmp99 = getelementptr inbounds %struct.patatino, %struct.patatino* %tmp92
+  %tmp134 = getelementptr inbounds %struct.fluttershy, %struct.fluttershy* undef, i64 0, i32 0
+  %tmp135 = bitcast %struct.dipsy** %tmp134 to %struct.patatino**
+  %tmp136 = load %struct.patatino*, %struct.patatino** %tmp135, align 8
+  br label %bb90
+
+bb138:
+  %tmp139 = getelementptr inbounds %struct.patatino, %struct.patatino* %tmp136
+  br label %bb138
+}
+
+; CHECK-LABEL: tinkywinky
+; CHECK-NEXT: bb:
+; CHECK-NEXT:   br label %bb90
+; CHECK-NEXT
+; CHECK:  bb90:
+; CHECK:  %tmp91 = bitcast %struct.dipsy** undef to %struct.patatino**
+; CHECK-NEXT:  %tmp92 = load %struct.patatino*, %struct.patatino** %tmp91, align 8
+; CHECK-NEXT:  %tmp136 = load %struct.patatino*, %struct.patatino** %tmp91, align 8
+; CHECK-NEXT:  br label %bb90
+; CHECK:  bb138:
+; CHECK-NEXT:  br label %bb138
+; CHECK-NEXT: }