Add -enable-ossa-simplifycfg and -enable-ossa-jumpthread-simplifycfg

as temporary flags to gradually stage in OSSA tests.

Author: atrick

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -203,7 +203,8 @@ SILLinkage getSpecializedLinkage(SILFunction *f, SILLinkage linkage);
 /// function \p Fn.
 bool tryCheckedCastBrJumpThreading(
     SILFunction *fn, DominanceInfo *dt,
-    SmallVectorImpl<SILBasicBlock *> &blocksForWorklist);
+    SmallVectorImpl<SILBasicBlock *> &blocksForWorklist,
+    bool EnableOSSARewriteTerminator);
 
 /// A utility for deleting one or more instructions belonging to a function, and
 /// cleaning up any dead code resulting from deleting those instructions. Use

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -43,6 +43,20 @@
 #include "llvm/Support/Debug.h"
 using namespace swift;
 
+// This is temporarily used for testing until Swift 5.5 branches to reduce risk.
+llvm::cl::opt<bool> EnableOSSASimplifyCFG(
+    "enable-ossa-simplify-cfg",
+    llvm::cl::desc(
+        "Enable non-trivial OSSA simplify-cfg and simple jump threading "
+        "(staging)."));
+
+// This requires new OwnershipOptUtilities which aren't well tested yet.
+llvm::cl::opt<bool> EnableOSSARewriteTerminator(
+    "enable-ossa-rewriteterminator",
+    llvm::cl::desc(
+        "Enable OSSA simplify-cfg with non-trivial terminator rewriting "
+        "(staging)."));
+
 STATISTIC(NumBlocksDeleted, "Number of unreachable blocks removed");
 STATISTIC(NumBlocksMerged, "Number of blocks merged together");
 STATISTIC(NumJumpThreads, "Number of jumps threaded");
@@ -661,23 +675,16 @@ bool SimplifyCFG::dominatorBasedSimplify(DominanceAnalysis *DA) {
   // Get the dominator tree.
   DT = DA->get(&Fn);
 
+  if (!EnableOSSASimplifyCFG && Fn.hasOwnership())
+    return false;
+
   // Split all critical edges such that we can move code onto edges. This is
   // also required for SSA construction in dominatorBasedSimplifications' jump
   // threading. It only splits new critical edges it creates by jump threading.
   bool Changed = false;
   if (!Fn.hasOwnership() && EnableJumpThread) {
     Changed = splitAllCriticalEdges(Fn, DT, nullptr);
   }
-
-  // TODO: OSSA phi support. Even if all block arguments are trivial,
-  // jump-threading may require creation of guaranteed phis, which may require
-  // creation of nested borrow scopes.
-  if (Fn.hasOwnership()) {
-    for (auto &BB : Fn)
-      Changed |= simplifyArgs(&BB);
-    return Changed;
-  }
-
   unsigned MaxIter = MaxIterationsOfDominatorBasedSimplify;
   SmallVector<SILBasicBlock *, 16> BlocksForWorklist;
 
@@ -688,7 +695,8 @@ bool SimplifyCFG::dominatorBasedSimplify(DominanceAnalysis *DA) {
     // Do dominator based simplification of terminator condition. This does not
     // and MUST NOT change the CFG without updating the dominator tree to
     // reflect such change.
-    if (tryCheckedCastBrJumpThreading(&Fn, DT, BlocksForWorklist)) {
+    if (tryCheckedCastBrJumpThreading(&Fn, DT, BlocksForWorklist,
+                                      EnableOSSARewriteTerminator)) {
       for (auto BB: BlocksForWorklist)
         addToWorklist(BB);
 
@@ -1029,10 +1037,13 @@ static bool hasInjectedEnumAtEndOfBlock(SILBasicBlock *block, SILValue enumAddr)
 /// tryJumpThreading - Check to see if it looks profitable to duplicate the
 /// destination of an unconditional jump into the bottom of this block.
 bool SimplifyCFG::tryJumpThreading(BranchInst *BI) {
+  if (!EnableOSSASimplifyCFG && Fn.hasOwnership())
+    return false;
+
   auto *DestBB = BI->getDestBB();
   auto *SrcBB = BI->getParent();
   TermInst *destTerminator = DestBB->getTerminator();
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     if (llvm::any_of(DestBB->getArguments(), [this](SILValue op) {
           return !op->getType().isTrivial(Fn);
         })) {
@@ -1821,8 +1832,7 @@ static bool isOnlyUnreachable(SILBasicBlock *BB) {
 /// switch_enum where all but one block consists of just an
 /// "unreachable" with an unchecked_enum_data and branch.
 bool SimplifyCFG::simplifySwitchEnumUnreachableBlocks(SwitchEnumInst *SEI) {
-  if (Fn.hasOwnership()) {
-    // TODO: OSSA; cleanup terminator results.
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     if (!SEI->getOperand()->getType().isTrivial(Fn))
       return false;
   }
@@ -2119,7 +2129,7 @@ static bool hasSameUltimateSuccessor(SILBasicBlock *noneBB, SILBasicBlock *someB
 bool SimplifyCFG::simplifySwitchEnumOnObjcClassOptional(SwitchEnumInst *SEI) {
   // TODO: OSSA; handle non-trivial enum case cleanup
   // (simplify_switch_enum_objc.sil).
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     return false;
   }
 
@@ -2183,7 +2193,7 @@ bool SimplifyCFG::simplifySwitchEnumBlock(SwitchEnumInst *SEI) {
   auto *LiveBlock = SEI->getCaseDestination(EnumCase.get());
   auto *ThisBB = SEI->getParent();
 
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     // TODO: OSSA; cleanup terminator results.
     if (!SEI->getOperand()->getType().isTrivial(Fn))
       return false;
@@ -2423,7 +2433,7 @@ bool SimplifyCFG::simplifyCheckedCastBranchBlock(CheckedCastBranchInst *CCBI) {
 bool SimplifyCFG::simplifyCheckedCastValueBranchBlock(
     CheckedCastValueBranchInst *CCBI) {
   // TODO: OSSA; handle cleanups for opaque cases (simplify_cfg_opaque.sil).
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     return false;
   }
 
@@ -2701,7 +2711,7 @@ bool SimplifyCFG::simplifyTermWithIdenticalDestBlocks(SILBasicBlock *BB) {
   TermInst *Term = BB->getTerminator();
   // TODO: OSSA; cleanup nontrivial terminator operands (if this ever actually
   // happens)
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     if (llvm::any_of(Term->getOperandValues(), [this](SILValue op) {
           return !op->getType().isTrivial(Fn);
         })) {
@@ -2896,7 +2906,7 @@ bool SimplifyCFG::simplifyBlocks() {
 /// Canonicalize all switch_enum and switch_enum_addr instructions.
 /// If possible, replace the default with the corresponding unique case.
 bool SimplifyCFG::canonicalizeSwitchEnums() {
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     // TODO: OSSA. In OSSA, the default switch_enum case passes the
     // original enum as a block argument. This needs to check that the block
     // argument is dead, then replace it with the a new argument for the default
@@ -3004,7 +3014,7 @@ bool SimplifyCFG::tailDuplicateObjCMethodCallSuccessorBlocks() {
   // TODO: OSSA phi support. Even if all block arguments are trivial,
   // jump-threading may require creation of guaranteed phis, which may require
   // creation of nested borrow scopes.
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     return false;
   }
   // Collect blocks to tail duplicate.
@@ -3213,7 +3223,7 @@ RemoveDeadArgsWhenSplitting("sroa-args-remove-dead-args-after",
                             llvm::cl::init(true));
 
 bool ArgumentSplitter::split() {
-  if (Arg->getFunction()->hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Arg->getFunction()->hasOwnership()) {
     // TODO: OSSA phi support
     if (!Arg->getType().isTrivial(*Arg->getFunction()))
       return false;
@@ -4005,7 +4015,7 @@ bool SimplifyCFG::simplifyArgs(SILBasicBlock *BB) {
   if (BB->pred_empty())
     return false;
 
-  if (Fn.hasOwnership()) {
+  if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
     // TODO: OSSA phi support
     if (llvm::any_of(BB->getArguments(), [this](SILArgument *arg) {
           return !arg->getType().isTrivial(Fn);

lib/SILOptimizer/Utils/CheckedCastBrJumpThreading.cpp

@@ -42,6 +42,9 @@ class CheckedCastBrJumpThreading {
   // Dominator information to be used.
   DominanceInfo *DT;
 
+  // Enable non-trivial terminator rewriting in OSSA.
+  bool EnableOSSARewriteTerminator;
+
   // List of predecessors.
   typedef SmallVector<SILBasicBlock *, 8> PredList;
 
@@ -119,10 +122,14 @@ class CheckedCastBrJumpThreading {
   bool trySimplify(CheckedCastBranchInst *CCBI);
 
 public:
-  CheckedCastBrJumpThreading(SILFunction *Fn, DominanceInfo *DT,
-                             SmallVectorImpl<SILBasicBlock *> &BlocksForWorklist)
-      : Fn(Fn), DT(DT), BlocksForWorklist(BlocksForWorklist),
-        BlocksToEdit(Fn), BlocksToClone(Fn) { }
+  CheckedCastBrJumpThreading(
+      SILFunction *Fn, DominanceInfo *DT,
+      SmallVectorImpl<SILBasicBlock *> &BlocksForWorklist,
+      bool EnableOSSARewriteTerminator)
+      : Fn(Fn), DT(DT),
+        EnableOSSARewriteTerminator(EnableOSSARewriteTerminator),
+        BlocksForWorklist(BlocksForWorklist), BlocksToEdit(Fn),
+        BlocksToClone(Fn) {}
 
   void optimizeFunction();
 };
@@ -486,6 +493,11 @@ areEquivalentConditionsAlongPaths(CheckedCastBranchInst *DomCCBI) {
 /// Try performing a dominator-based jump-threading for
 /// checked_cast_br instructions.
 bool CheckedCastBrJumpThreading::trySimplify(CheckedCastBranchInst *CCBI) {
+  if (!EnableOSSARewriteTerminator && Fn->hasOwnership()
+      && !CCBI->getOperand()->getType().isTrivial(*Fn)) {
+    return false;
+  }
+
   // Init information about the checked_cast_br we try to
   // jump-thread.
   BB = CCBI->getParent();
@@ -685,9 +697,13 @@ void CheckedCastBrJumpThreading::optimizeFunction() {
 
 namespace swift {
 
-bool tryCheckedCastBrJumpThreading(SILFunction *Fn, DominanceInfo *DT,
-                        SmallVectorImpl<SILBasicBlock *> &BlocksForWorklist) {
-  CheckedCastBrJumpThreading CCBJumpThreading(Fn, DT, BlocksForWorklist);
+bool tryCheckedCastBrJumpThreading(
+    SILFunction *Fn, DominanceInfo *DT,
+    SmallVectorImpl<SILBasicBlock *> &BlocksForWorklist,
+    bool EnableOSSARewriteTerminator) {
+
+  CheckedCastBrJumpThreading CCBJumpThreading(Fn, DT, BlocksForWorklist,
+                                              EnableOSSARewriteTerminator);
   CCBJumpThreading.optimizeFunction();
   return !BlocksForWorklist.empty();
 }