[PATCH] D73285: [OpenMP][OMPIRBuilder][BugFix] Handle Unreachable Finalization blocks in `parallel` generation

[Fady Ghanim via Phabricator via cfe-commits]

fghanim updated this revision to Diff 239991.
fghanim added a comment.

- Cleaning up some leftover code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73285/new/

https://reviews.llvm.org/D73285

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===================================================================
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -439,6 +439,18 @@
         Worklist.push_back(SuccBB);
   }
 
+  // If we didn't emit a branch to FiniBB during body generation, it means
+  // FiniBB is unreachable (e.g. while(1);). stop generating all the
+  // unreachable blocks, and remove anything we are not going to use.
+  // Check to see if PRegPreFiniBB is reachable from PRegionBodyBB.
+  bool FoundPreFiniBB = false;
+  for (auto BI : ParallelRegionBlocks) {
+    if (BI == PRegPreFiniBB) {
+      FoundPreFiniBB = true;
+      break;
+    }
+  }
+
   CodeExtractorAnalysisCache CEAC(*OuterFn);
   CodeExtractor Extractor(ParallelRegionBlocks, /* DominatorTree */ nullptr,
                           /* AggregateArgs */ false,
@@ -564,7 +576,7 @@
     }
   }
 
-  Builder.CreateCall(RTLFn, RealArgs);
+  CallInst *ForkCall = Builder.CreateCall(RTLFn, RealArgs);
 
   LLVM_DEBUG(dbgs() << "With fork_call placed: "
                     << *Builder.GetInsertBlock()->getParent() << "\n");
@@ -583,7 +595,6 @@
   if (!ElseTI) {
     CI->eraseFromParent();
   } else {
-
     // If an "if" clause was present we are now generating the serialized
     // version into the "else" branch.
     Builder.SetInsertPoint(ElseTI);
@@ -608,26 +619,53 @@
                       << *Builder.GetInsertBlock()->getParent() << "\n");
   }
 
-  // Adjust the finalization stack, verify the adjustment, and call the
-  // finalize function a last time to finalize values between the pre-fini block
-  // and the exit block if we left the parallel "the normal way".
+  assert(!FinalizationStack.empty() && "Unexpected finalization stack state!");
   auto FiniInfo = FinalizationStack.pop_back_val();
-  (void)FiniInfo;
   assert(FiniInfo.DK == OMPD_parallel &&
          "Unexpected finalization stack state!");
+  if (FoundPreFiniBB) {
+    // PRegPreFiniBB is reachable. Adjust the finalization stack, verify the
+    // adjustment, and call the finalize function a last time to finalize values
+    // between the pre-fini block and the exit block if we left the parallel
+    // "the normal way".
+    (void)FiniInfo;
+
+    Instruction *PreFiniTI = PRegPreFiniBB->getTerminator();
+    assert(PreFiniTI->getNumSuccessors() == 1 &&
+           PreFiniTI->getSuccessor(0)->size() == 1 &&
+           isa<ReturnInst>(PreFiniTI->getSuccessor(0)->getTerminator()) &&
+           "Unexpected CFG structure!");
+
+    InsertPointTy PreFiniIP(PRegPreFiniBB, PreFiniTI->getIterator());
+    FiniCB(PreFiniIP);
+  } else {
+    // PRegPreFiniBB is unreachable. remove the unreachable blocks
+    // and discard the finalization callback
+    llvm::SmallVector<BasicBlock *, 4> ToBeDeletedBB;
+    ToBeDeletedBB.push_back(PRegPreFiniBB);
+    BranchInst *BBTerminator =
+        dyn_cast_or_null<BranchInst>(PRegPreFiniBB->getTerminator());
+    while (BBTerminator) {
+      assert(!BBTerminator->isConditional() &&
+             "unexpected conditional branch in unreachable blocks");
+      BasicBlock *next = BBTerminator->getSuccessor(0);
+      ToBeDeletedBB.push_back(next);
+      BBTerminator = dyn_cast_or_null<BranchInst>(next->getTerminator());
+    }
 
-  Instruction *PreFiniTI = PRegPreFiniBB->getTerminator();
-  assert(PreFiniTI->getNumSuccessors() == 1 &&
-         PreFiniTI->getSuccessor(0)->size() == 1 &&
-         isa<ReturnInst>(PreFiniTI->getSuccessor(0)->getTerminator()) &&
-         "Unexpected CFG structure!");
+    for (auto BB : ToBeDeletedBB) {
+      BB->eraseFromParent();
+    }
 
-  InsertPointTy PreFiniIP(PRegPreFiniBB, PreFiniTI->getIterator());
-  FiniCB(PreFiniIP);
+    BasicBlock *ForkBB = ForkCall->getParent();
+    ForkBB->getTerminator()->eraseFromParent();
+    AfterIP = InsertPointTy(ForkBB, ForkBB->end());
+  }
 
   for (Instruction *I : ToBeDeleted)
     I->eraseFromParent();
 
+  AfterIP.getBlock()->dump();
   return AfterIP;
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===================================================================
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1377,15 +1377,15 @@
       ReturnBlock = getJumpDestInCurrentScope(&ContinuationBB);
 
       llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
-      CodeGenIPBB->splitBasicBlock(CodeGenIP.getPoint());
       llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator();
-      CodeGenIPBBTI->removeFromParent();
+      CodeGenIPBBTI->eraseFromParent();
 
       Builder.SetInsertPoint(CodeGenIPBB);
 
       EmitStmt(ParallelRegionBodyStmt);
 
-      Builder.Insert(CodeGenIPBBTI);
+      if (Builder.saveIP().isSet())
+        Builder.CreateBr(&ContinuationBB);
 
       AllocaInsertPt = OldAllocaIP;
       ReturnBlock = OldReturnBlock;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits