[PredicateInfo] Do not process unreachable operands.

[twoh]

Summary

We should excluded unreachable operands from processing as their DFS visitation order is undefined. When renameUses function sorts OpsToRename (https://fburl.com/d2wubn60), the comparator assumes that the parent block of the operand has a corresponding dominator tree node. This is not the case for unreachable operands and crashes the compiler.

[twoh]

Friendly ping. Thanks!

[twoh]

Friendly ping again. This is a fix for a compiler crash.

[efriedma]

Testcase?

For any non-PHI instruction that is reachable, all of its operands must also be reachable. We can use this to reduce the number of places we need to check for reachability. I'm pretty sure the new checks in processSwitch and processBranch are useless: iterating over the domtree will not visit unreachable blocks. And we should probably add a check to PredicateInfo::buildPredicateInfo to avoid processing assumes in unreachable code, rather than checking the reachability of the operands of an assume.

[twoh]

Thank you for your comments @efriedma! I addressed your comments and added a test case. The test segfaults clang without proposed change.

[efriedma]

LGTM with one minor comment.

llvm/lib/Transforms/Utils/PredicateInfo.cpp

476–477

It would be more clear to use isReachableFromEntry (which does the same thing, but it has a better name).

[twoh]

Use isReachableFromEntry. Thank you for the suggestion!

llvm/lib/Transforms/Utils/PredicateInfo.cpp

if (auto *BI = dyn_cast<BranchInst>(BranchBB->getTerminator())) {
         continue;
       processBranch(BI, BranchBB, OpsToRename);
     } else if (auto *SI = dyn_cast<SwitchInst>(BranchBB->getTerminator())) {
       processSwitch(SI, BranchBB, OpsToRename);
     }
   }
   for (auto &Assume : AC.assumptions()) {
     if (auto *II = dyn_cast_or_null<IntrinsicInst>(Assume))
+      if (DT.isReachableFromEntry(II->getParent()))
         processAssume(II, II->getParent(), OpsToRename);
   }
   // Now rename all our operations.
   renameUses(OpsToRename);
 }
 
 // Create a ssa_copy declaration with custom mangling, because
 // Intrinsic::getDeclaration does not handle overloaded unnamed types properly:
 // all unnamed types get mangled to the same string. We use the pointer

llvm/test/Transforms/Util/PredicateInfo/unreachable.ll

+; RUN: opt -print-predicateinfo < %s 2>&1 | FileCheck %s
+
+declare void @foo()
+declare void @llvm.assume(i1)
+
+define void @bar(i32* %p) {
+entry:
+; CHECK-LABEL: @bar
+  br label %end
+
+unreachable1:
+  %v1 = load i32, i32* %p, align 4
+  %c1 = icmp eq i32 %v1, 0
+  call void @llvm.assume(i1 %c1)
+  br label %unreachable2
+
+unreachable2:
+  %v2 = load i32, i32* %p, align 4
+  %c2 = icmp eq i32 %v2, 0
+  call void @llvm.assume(i1 %c2)
+  br label %end
+
+end:
+  ret void
+}