Merge pull request swiftlang#34500 from meg-gupta/fixmem2regassert

SILMem2Reg : Delete phi args added by Mem2Reg if there are no uses

Author: meg-gupta

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

@@ -573,12 +573,8 @@ StackAllocationPromoter::promoteAllocationInBlock(SILBasicBlock *BB) {
 
     // Stop on deallocation.
     if (auto *DSI = dyn_cast<DeallocStackInst>(Inst)) {
-      if (DSI->getOperand() == ASI) {
-        // Reset LastStore.
-        // So that we don't pass RunningVal as a phi arg beyond dealloc_stack
-        LastStore = nullptr;
+      if (DSI->getOperand() == ASI)
         break;
-      }
     }
   }
   if (LastStore) {
@@ -756,21 +752,6 @@ void StackAllocationPromoter::fixPhiPredBlock(BlockSet &PhiBlocks,
   TI->eraseFromParent();
 }
 
-static bool hasOnlyUndefIncomingValues(SILPhiArgument *phiArg) {
-  SmallVector<SILValue, 8> incomingValues;
-  phiArg->getIncomingPhiValues(incomingValues);
-  for (auto predArg : incomingValues) {
-    if (isa<SILUndef>(predArg))
-      continue;
-    if (isa<SILPhiArgument>(predArg) &&
-        hasOnlyUndefIncomingValues(cast<SILPhiArgument>(predArg))) {
-      continue;
-    }
-    return false;
-  }
-  return true;
-}
-
 void StackAllocationPromoter::fixBranchesAndUses(BlockSet &PhiBlocks) {
   // First update uses of the value.
   SmallVector<LoadInst *, 4> collectedLoads;
@@ -852,18 +833,13 @@ void StackAllocationPromoter::fixBranchesAndUses(BlockSet &PhiBlocks) {
   // If the owned phi arg we added did not have any uses, create end_lifetime to
   // end its lifetime. In asserts mode, make sure we have only undef incoming
   // values for such phi args.
-  if (ASI->getFunction()->hasOwnership()) {
     for (auto Block : PhiBlocks) {
       auto *phiArg = cast<SILPhiArgument>(
           Block->getArgument(Block->getNumArguments() - 1));
-      if (phiArg->getOwnershipKind() == ValueOwnershipKind::Owned &&
-          phiArg->use_empty()) {
-        assert(hasOnlyUndefIncomingValues(phiArg));
-        SILBuilderWithScope(&Block->front())
-            .createEndLifetime(Block->front().getLoc(), phiArg);
+      if (phiArg->use_empty()) {
+        erasePhiArgument(Block, Block->getNumArguments() - 1);
       }
     }
-  }
 }
 
 void StackAllocationPromoter::pruneAllocStackUsage() {

test/SILOptimizer/mem2reg.sil

@@ -466,10 +466,10 @@ bb0(%0 : $Optional<Klass>):
   return %4 : $()
 }
 
+// check no dead args are passed to bb3
 // CHECK-LABEL: sil @multi_basic_block_use_on_one_path :
 // CHECK-NOT: alloc_stack
-// CHECK:bb2:
-// CHECK: br bb3(undef : $Klass)
+// CHECK: bb3:
 // CHECK-LABEL: } // end sil function 'multi_basic_block_use_on_one_path'
 sil @multi_basic_block_use_on_one_path : $@convention(thin) (@owned Klass) -> () {
 bb0(%0 : $Klass):

test/SILOptimizer/mem2reg_ossa_nontrivial.sil

@@ -384,12 +384,12 @@ bb3:
   return %res : $()
 }
 
+// Test to check no dead args are passed to bb3 as phi arg
 // CHECK-LABEL: sil [ossa] @multi_basic_block_stack_deallocated_phiarg :
 // CHECK-NOT: alloc_stack
 // CHECK-LABEL: bb2:
-// CHECK: br bb3(undef : $Klass)
-// CHECK: bb3([[PHI:%.*]] : @owned $Klass):
-// CHECK-NEXT:  end_lifetime [[PHI]] : $Klass
+// CHECK: br bb3
+// CHECK: bb3:
 // CHECK-LABEL: } // end sil function 'multi_basic_block_stack_deallocated_phiarg'
 sil [ossa] @multi_basic_block_stack_deallocated_phiarg : $@convention(thin) (@owned Klass) -> () {
 bb0(%0 : @owned $Klass):
@@ -410,13 +410,12 @@ bb3:
   return %res : $()
 }
 
+// Test to check no dead args are passed to bb3 as phi arg
 // CHECK-LABEL: sil [ossa] @multi_asi_basic_block_stack_deallocated_phiarg :
 // CHECK-NOT: alloc_stack
 // CHECK-LABEL: bb2:
-// CHECK: br bb3(undef : $Klass, undef : $Klass)
-// CHECK: bb3([[PHI1:%.*]] : @owned $Klass, [[PHI2:%.*]] : @owned $Klass):
-// CHECK-NEXT:  end_lifetime [[PHI2]] : $Klass
-// CHECK-NEXT:  end_lifetime [[PHI1]] : $Klass
+// CHECK: br bb3
+// CHECK: bb3:
 // CHECK-LABEL: } // end sil function 'multi_asi_basic_block_stack_deallocated_phiarg'
 sil [ossa] @multi_asi_basic_block_stack_deallocated_phiarg : $@convention(thin) (@owned Klass, @owned Klass) -> () {
 bb0(%0 : @owned $Klass, %1 : @owned $Klass):
@@ -446,8 +445,7 @@ bb3:
 
 // CHECK-LABEL: sil [ossa] @multi_basic_block_destroyed_last_stored_val_phiarg :
 // CHECK-NOT: alloc_stack
-// CHECK-LABEL: bb2:
-// CHECK: br bb3(undef : $Klass)
+// CHECK-LABEL: bb3:
 // CHECK-LABEL: } // end sil function 'multi_basic_block_destroyed_last_stored_val_phiarg'
 sil [ossa] @multi_basic_block_destroyed_last_stored_val_phiarg : $@convention(thin) (@owned Klass) -> () {
 bb0(%0 : @owned $Klass):
@@ -630,3 +628,51 @@ bb0(%0 : @owned $Klass):
   return %7 : $()
 }
 
+// CHECK-LABEL: sil [ossa] @multi_basic_block_bug1 :
+// CHECK-NOT: alloc_stack
+// CHECK-LABEL: } // end sil function 'multi_basic_block_bug1'
+sil [ossa] @multi_basic_block_bug1 : $@convention(thin) (@owned Klass, @owned Klass) -> @owned Klass {
+bb0(%0 : @owned $Klass, %1 : @owned $Klass):
+  %stk1 = alloc_stack $Klass
+  store %0 to [init] %stk1 : $*Klass
+  cond_br undef, bb1, bb2
+
+bb1:
+  %new1 = load [take] %stk1 : $*Klass
+  destroy_value %1 : $Klass
+  dealloc_stack %stk1 : $*Klass
+  br bbret(%new1 : $Klass)
+
+bb2:
+  store %1 to [assign] %stk1 : $*Klass
+  %new2 = load [take] %stk1 : $*Klass
+  dealloc_stack %stk1 : $*Klass
+  br bbret(%new2 : $Klass)
+
+bbret(%new : @owned $Klass):
+  return %new : $Klass
+}
+
+// CHECK-LABEL: sil [ossa] @multi_basic_block_bug2 :
+// CHECK-NOT: alloc_stack
+// CHECK-LABEL: } // end sil function 'multi_basic_block_bug2'
+sil [ossa] @multi_basic_block_bug2 : $@convention(thin) (@owned Klass, @owned Klass) -> @owned Klass {
+bb0(%0 : @owned $Klass, %1 : @owned $Klass):
+  %stk1 = alloc_stack $Klass
+  store %0 to [init] %stk1 : $*Klass
+  cond_br undef, bb1, bb2
+
+bb1:
+  %new1 = load [take] %stk1 : $*Klass
+  destroy_value %1 : $Klass
+  br bbret(%new1 : $Klass)
+
+bb2:
+  store %1 to [assign] %stk1 : $*Klass
+  %new2 = load [take] %stk1 : $*Klass
+  br bbret(%new2 : $Klass)
+
+bbret(%new : @owned $Klass):
+  dealloc_stack %stk1 : $*Klass
+  return %new : $Klass
+}