[LoopDeletion] Don't delete loops with blocks that have their address taken

[aeubanks]

Summary

Otherwise we may end up making the blockaddress invalid and passing it to other functions.

[nickdesaulniers]

Should we be replacing the blockaddress with a Constant 1 in such cases? We do that elsewhere in a couple places. @efriedma ?

[nickdesaulniers]

In D123870#3454609, @nickdesaulniers wrote:

Should we be replacing the blockaddress with a Constant 1 in such cases? We do that elsewhere in a couple places. @efriedma ?

Oh, that's what the previous behavior of llvm/test/Transforms/LoopDeletion/blockaddress.ll was testing.

[nickdesaulniers]

In D123870#3454612, @nickdesaulniers wrote:

In D123870#3454609, @nickdesaulniers wrote:

Should we be replacing the blockaddress with a Constant 1 in such cases? We do that elsewhere in a couple places. @efriedma ?

Oh, that's what the previous behavior of llvm/test/Transforms/LoopDeletion/blockaddress.ll was testing.

Ah, this test was precommitted recently: https://github.com/llvm/llvm-project/commit/4d85859ff48d16fc0ca8199676f7417a2c6dfe43

[efriedma]

In general, transformations are allowed to delete unreachable blocks that have their address taken. See https://llvm.org/docs/LangRef.html#addresses-of-basic-blocks etc. So I don't see a bug here.

[nickdesaulniers]

In D123870#3454628, @efriedma wrote:

In general, transformations are allowed to delete unreachable blocks that have their address taken. See https://llvm.org/docs/LangRef.html#addresses-of-basic-blocks etc. So I don't see a bug here.

That's what I thought. Throwing a gentle -1 on this until convinced otherwise.

[aeubanks]

https://crsrc.org/c/base/debug/stack_trace.cc;drc=dd12c1c88dda05cccfb2bf9b9c9341ca8bdb3d1b;l=395

we're comparing the pc against two labels in a function to maybe skip stack frames. I guess label addresses aren't meant to do that? http://theochem.ki.ku.dk/on_line_docs/gcc/gcc_4.html suggests that they're only used for computed gotos?

llvm/lib/Transforms/Scalar/LoopDeletion.cpp

 static LoopDeletionResult merge(LoopDeletionResult A, LoopDeletionResult B) {
   if (A == LoopDeletionResult::Deleted || B == LoopDeletionResult::Deleted)
     return LoopDeletionResult::Deleted;
   if (A == LoopDeletionResult::Modified || B == LoopDeletionResult::Modified)
     return LoopDeletionResult::Modified;
   return LoopDeletionResult::Unmodified;
 }
 
+static bool loopHasAddressTaken(Loop *L) {
+  return any_of(L->blocks(),
+                [](BasicBlock *BB) { return BB->hasAddressTaken(); });
+}
+
 /// Determines if a loop is dead.
 ///
 /// This assumes that we've already checked for unique exit and exiting blocks,
 /// and that the code is in LCSSA form.
 static bool isLoopDead(Loop *L, ScalarEvolution &SE,
                        SmallVectorImpl<BasicBlock *> &ExitingBlocks,
                        BasicBlock *ExitBlock, bool &Changed,
                        BasicBlock *Preheader, LoopInfo &LI) {
static LoopDeletionResult deleteLoopIfDead(Loop *L, DominatorTree &DT,
   BasicBlock *Preheader = L->getLoopPreheader();
   if (!Preheader || !L->hasDedicatedExits()) {
     LLVM_DEBUG(
         dbgs()
         << "Deletion requires Loop with preheader and dedicated exits.\n");
     return LoopDeletionResult::Unmodified;
   }
 
+  if (loopHasAddressTaken(L)) {
+    LLVM_DEBUG(dbgs() << "Loop has address taken, cannot delete.\n");
+    return LoopDeletionResult::Unmodified;
+  }
+
   BasicBlock *ExitBlock = L->getUniqueExitBlock();
 
   if (ExitBlock && isLoopNeverExecuted(L)) {
     LLVM_DEBUG(dbgs() << "Loop is proven to never execute, delete it!");
     // We need to forget the loop before setting the incoming values of the exit
     // phis to undef, so we properly invalidate the SCEV expressions for those
     // phis.
     SE.forgetLoop(L);

llvm/test/Transforms/LoopDeletion/blockaddress.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
 ; RUN: opt -passes=loop-deletion -S < %s | FileCheck %s
 
 @ba = private constant ptr blockaddress(@f, %bb)
 
 declare void @g(ptr)
 
 ;.
-; CHECK: @[[BA:[a-zA-Z0-9_$"\\.-]+]] = private constant ptr inttoptr (i32 1 to ptr)
+; CHECK: @[[BA:[a-zA-Z0-9_$"\\.-]+]] = private constant ptr blockaddress(@f, [[BB:%.*]])
 ;.
 define void @f() {
 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[BB:%.*]]
+; CHECK:       bb:
 ; CHECK-NEXT:    br label [[BB2:%.*]]
 ; CHECK:       bb2:
 ; CHECK-NEXT:    call void @g(ptr @ba)
 ; CHECK-NEXT:    ret void
 ;
 entry:
   br label %bb
 bb:
   br i1 true, label %bb2, label %bb
 bb2:
   call void @g(ptr @ba)
   ret void
 }