[PATCH] D81494: [GVN] Do not try to split critical edges with indbr preds in loop.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: reames, hfinkel, davide, efriedma.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

splitCriticalEdges may crash while updating LI in case one of the
predecessors has a indbr and is in a different loop than the
predecessor, because it cannot split blocks with indbr terminators.

This crash was exposed by 189efe295b6e.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81494

Files:
  llvm/lib/Transforms/Scalar/GVN.cpp
  llvm/test/Transforms/GVN/critical-edge-split-indbr-pred-in-loop.ll


Index: llvm/test/Transforms/GVN/critical-edge-split-indbr-pred-in-loop.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/GVN/critical-edge-split-indbr-pred-in-loop.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -gvn -S -verify-loop-info %s | FileCheck %s
+
+; Make sure we do not try to split blocks with indirectbr terminators in a
+; loop while trying to preserve loop info.
+
+declare i1 @cond()
+
+define i32 @foo(i8* %p1, i8* %p2, i32* %p3) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[LOOP_HEADER:%.*]]
+; CHECK:       loop.header:
+; CHECK-NEXT:    store i32 0, i32* [[P3:%.*]], align 4
+; CHECK-NEXT:    indirectbr i8* [[P1:%.*]], [label [[LOOP_LATCH1:%.*]], label %loop.latch2]
+; CHECK:       loop.latch1:
+; CHECK-NEXT:    [[C:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[C]], label [[LOOP_HEADER]], label [[EXIT:%.*]]
+; CHECK:       loop.latch2:
+; CHECK-NEXT:    indirectbr i8* [[P2:%.*]], [label [[LOOP_HEADER]], label %exit]
+; CHECK:       exit:
+; CHECK-NEXT:    [[X:%.*]] = load i32, i32* [[P3]], align 4
+; CHECK-NEXT:    ret i32 [[X]]
+;
+bb:
+  br label %loop.header
+
+loop.header:                                      ; preds = %loop.latch2, %loop.latch1, %bb
+  store i32 0, i32* %p3, align 4
+  indirectbr i8* %p1, [label %loop.latch1, label %loop.latch2]
+
+loop.latch1:                                      ; preds = %loop.header
+  %c = call i1 @cond()
+  br i1 %c, label %loop.header, label %exit
+
+loop.latch2:                                      ; preds = %loop.header
+  indirectbr i8* %p2, [label %loop.header, label %exit]
+
+exit:                                             ; preds = %loop.latch2, %loop.latch1
+  %x = load i32, i32* %p3, align 4
+  %y = add i32 %x, 0
+  ret i32 %y
+}
Index: llvm/lib/Transforms/Scalar/GVN.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/GVN.cpp
+++ llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1114,6 +1114,13 @@
     FullyAvailableBlocks[UnavailableBB] = false;
 
   SmallVector<BasicBlock *, 4> CriticalEdgePred;
+
+  bool HasLI = this->LI;
+  // Keep track if there are predecessors in a different loop than LoadBB with
+  // indbr terminators. We cannot use splitCriticalEdges, as we might have to
+  // split a predecessor with an indbr, which is not supported.
+  Loop *LoadBBL = HasLI ? this->LI->getLoopFor(LoadBB) : nullptr;
+  bool PredHasIndirectBr = false;
   for (BasicBlock *Pred : predecessors(LoadBB)) {
     // If any predecessor block is an EH pad that does not allow non-PHI
     // instructions before the terminator, we can't PRE the load.
@@ -1124,6 +1131,9 @@
       return false;
     }
 
+    PredHasIndirectBr |= HasLI ? this->LI->getLoopFor(Pred) != LoadBBL &&
+                                     isa<IndirectBrInst>(Pred->getTerminator())
+                               : false;
     if (IsValueFullyAvailableInBlock(Pred, FullyAvailableBlocks, 0)) {
       continue;
     }
@@ -1170,6 +1180,11 @@
   if (NumUnavailablePreds != 1)
       return false;
 
+  if (PredHasIndirectBr) {
+    LLVM_DEBUG(dbgs() << "COULD NOT PRE LOAD BECAUSE INDBR EDGE IN LOOP\n");
+    return false;
+  }
+
   // Split critical edges, and update the unavailable predecessors accordingly.
   for (BasicBlock *OrigPred : CriticalEdgePred) {
     BasicBlock *NewPred = splitCriticalEdges(OrigPred, LoadBB);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81494.269625.patch
Type: text/x-patch
Size: 3490 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200609/c2d7b91b/attachment.bin>