[Local] Zap blockaddress without users in ConstantFoldTerminator.

[fhahn]

Summary

If the blockaddress is not destoryed, the destination block will still
be marked as having its address taken, limiting further transformations.

I think there are other places where the dead blockaddress constants are kept
around, I'll look into that as follow up.

[grandinj]

Just a comment from a bystander:

Isn't there some kind of verifier pass that clang runs (perhaps under some expensive-checks flag) after every pass?

If such a thing exists, checking that BlockAddress nodes are properly cleaned seems like something it should do.

[fhahn]

In D64936#1593013, @grandinj wrote:

Just a comment from a bystander:

Isn't there some kind of verifier pass that clang runs (perhaps under some expensive-checks flag) after every pass?

If such a thing exists, checking that BlockAddress nodes are properly cleaned seems like something it should do.

Excellent suggestion: I've added a check to the verifier (D64984) and it highlighted 2 other places where we need additional cleanups.

[brzycki]

LGTM. I am assuming this passed test-suite and ninja check runs?

[fhahn]

In D64936#1593918, @brzycki wrote:

LGTM. I am assuming this passed test-suite and ninja check runs?

Yep, thanks!

llvm/lib/Transforms/Utils/Local.cpp

if (auto *BA =
           DestBB->removePredecessor(ParentBB);
           if (DTU)
             Updates.push_back({DominatorTree::Delete, ParentBB, DestBB});
         }
       }
       Value *Address = IBI->getAddress();
       IBI->eraseFromParent();
       if (DeleteDeadConditions)
+        // Delete pointer cast instructions.
         RecursivelyDeleteTriviallyDeadInstructions(Address, TLI);
 
+      // Also zap the blockaddress constant if there are no users remaining,
+      // otherwise the destination is still marked as having its address taken.
+      if (BA->use_empty())
+        BA->destroyConstant();
+
       // If we didn't find our destination in the IBI successor list, then we
       // have undefined behavior.  Replace the unconditional branch with an
       // 'unreachable' instruction.
       if (TheOnlyDest) {
         BB->getTerminator()->eraseFromParent();
         new UnreachableInst(BB->getContext(), BB);
       }
 

llvm/test/Transforms/SimplifyCFG/dce-cond-after-folding-terminator.ll

 sw.epilog:                                        ; preds = %sw.bb
   ret void
 }
 
 define void @test_indirectbr(i32 %x) {
 entry:
 ; CHECK-LABEL: @test_indirectbr(
 ; CHECK-NEXT: entry:
-; Ideally this should now check:
-;   CHK-NEXT: ret void
-; But that doesn't happen yet. Instead:
-; CHECK-NEXT: br label %L1
+; CHECK-NEXT:   ret void
 
   %label = bitcast i8* blockaddress(@test_indirectbr, %L1) to i8*
   indirectbr i8* %label, [label %L1, label %L2]
 
 L1:                                               ; preds = %entry
   ret void
 L2:                                               ; preds = %entry
   ret void
 }