IRGen: Fix out-of-order task_dealloc with parameter pack metadata

We deallocate an instruction's packs at points where no further
control flow path uses the value. In the case of an alloc_stack,
this will be right after the dealloc_stack. Thus, if alloc_stack
allocates some packs to build type metadata for a tuple type
that contains a pack, and then proceeds to allocate a value
large enough to hold the tuple, we will free the second allocation
first, before we free the pack, as expected.

However, after stack allocating the value, alloc_stack does
some further work to emit debug info. This could result in
emission of additional metadata packs.

In the cases I saw, these additional packs were duplicates, that
were only emitted because we do not canonicalize a bare
PackArchetypeType versus one wrapped in a singleton pack.

Add a narrow workaround to debug info to avoid this problem.
More generally, we should fix the canonicalization here, and
also come up with a more general bookkeeping mechanism for
stack allocations if it turns out that alloc_stack cannot
be refactored to always emit packs first.

Author: slavapestov

lib/IRGen/IRGenSIL.cpp

@@ -1158,12 +1158,10 @@ class IRGenSILFunction :
           }
 
           emitTypeMetadataRef(archetype);
-        } else if (auto packArchetype = dyn_cast<PackArchetypeType>(t)) {
-          emitTypeMetadataRef(packArchetype);
-        } else if (auto packtype = dyn_cast<SILPackType>(t)) {
+        } else if (auto packType = dyn_cast<SILPackType>(t)) {
           llvm::Value *Shape = emitPackShapeExpression(t);
           emitPackCountDebugVariable(Shape);
-        } else if (auto packtype = dyn_cast<PackType>(t)) {
+        } else if (auto packType = dyn_cast<PackType>(t)) {
           llvm::Value *Shape = emitPackShapeExpression(t);
           emitPackCountDebugVariable(Shape);
         }
@@ -1212,8 +1210,13 @@ class IRGenSILFunction :
                           SILResultInfo ErrorInfo,
                           DebugValueInst *DbgValue);
   void emitPoisonDebugValueInst(DebugValueInst *i);
-  void emitDebugInfoForAllocStack(AllocStackInst *i, const TypeInfo &type,
-                                  llvm::Value *addr);
+  void emitDebugInfoBeforeAllocStack(AllocStackInst *i,
+                                     const TypeInfo &type,
+                                     DebugTypeInfo &DbgTy);
+  void emitDebugInfoAfterAllocStack(AllocStackInst *i,
+                                    const TypeInfo &type,
+                                    const DebugTypeInfo &DbgTy,
+                                    llvm::Value *addr);
   void visitAllocStackInst(AllocStackInst *i);
   void visitAllocPackInst(AllocPackInst *i);
   void visitAllocPackMetadataInst(AllocPackMetadataInst *i);
@@ -6410,14 +6413,58 @@ void IRGenSILFunction::visitDestroyNotEscapedClosureInst(
   setLoweredExplosion(i, out);
 }
 
-void IRGenSILFunction::emitDebugInfoForAllocStack(AllocStackInst *i,
-                                                  const TypeInfo &type,
-                                                  llvm::Value *addr) {
+void IRGenSILFunction::emitDebugInfoBeforeAllocStack(AllocStackInst *i,
+                                                     const TypeInfo &type,
+                                                     DebugTypeInfo &DbgTy) {
   auto VarInfo = i->getVarInfo();
   if (!VarInfo)
     return;
 
+  auto DS = i->getDebugScope();
+  if (!DS)
+    return;
+
+  if (i->getDebugScope()->getInlinedFunction()->isTransparent())
+    return;
+
+  VarDecl *Decl = i->getDecl();
+
+  SILType SILTy;
+  if (auto MaybeSILTy = VarInfo->Type) {
+    // If there is auxiliary type info, use it
+    SILTy = *MaybeSILTy;
+  } else {
+    SILTy = i->getType();
+  }
+  auto RealType = SILTy.getASTType();
+  if (Decl) {
+    DbgTy = DebugTypeInfo::getLocalVariable(Decl, RealType, type, IGM);
+  } else if (i->getFunction()->isBare() && !SILTy.hasArchetype() &&
+             !VarInfo->Name.empty()) {
+    DbgTy = DebugTypeInfo::getFromTypeInfo(RealType, getTypeInfo(SILTy), IGM);
+  } else
+    return;
+
+  bindArchetypes(DbgTy.getType());
+}
+
+void IRGenSILFunction::emitDebugInfoAfterAllocStack(AllocStackInst *i,
+                                                    const TypeInfo &type,
+                                                    const DebugTypeInfo &DbgTy,
+                                                    llvm::Value *addr) {
+  auto VarInfo = i->getVarInfo();
+  if (!VarInfo)
+    return;
+
+  auto DS = i->getDebugScope();
+  if (!DS)
+    return;
+
+  if (i->getDebugScope()->getInlinedFunction()->isTransparent())
+    return;
+
   VarDecl *Decl = i->getDecl();
+
   // Describe the underlying alloca. This way an llvm.dbg.declare intrinsic
   // is used, which is valid for the entire lifetime of the alloca.
   if (auto *BitCast = dyn_cast<llvm::BitCastInst>(addr)) {
@@ -6434,13 +6481,6 @@ void IRGenSILFunction::emitDebugInfoForAllocStack(AllocStackInst *i,
       (void)isTaskAlloc;
     }
   }
-
-  auto DS = i->getDebugScope();
-  if (!DS)
-    return;
-
-  if (i->getDebugScope()->getInlinedFunction()->isTransparent())
-    return;
 
   bool IsAnonymous = false;
   VarInfo->Name = getVarName(i, IsAnonymous);
@@ -6475,25 +6515,15 @@ void IRGenSILFunction::emitDebugInfoForAllocStack(AllocStackInst *i,
     }
   }
 
-  SILType SILTy;
-  if (auto MaybeSILTy = VarInfo->Type) {
-    // If there is auxiliary type info, use it
-    SILTy = *MaybeSILTy;
-  } else {
-    SILTy = i->getType();
-  }
-  auto RealType = SILTy.getASTType();
-  DebugTypeInfo DbgTy;
-  if (Decl) {
-    DbgTy = DebugTypeInfo::getLocalVariable(Decl, RealType, type, IGM);
-  } else if (i->getFunction()->isBare() && !SILTy.hasArchetype() &&
-             !VarInfo->Name.empty()) {
-    DbgTy = DebugTypeInfo::getFromTypeInfo(RealType, getTypeInfo(SILTy), IGM);
-  } else
-    return;
+  if (DbgTy.getType() && IGM.DebugInfo) {
+    SILType SILTy;
+    if (auto MaybeSILTy = VarInfo->Type) {
+      // If there is auxiliary type info, use it
+      SILTy = *MaybeSILTy;
+    } else {
+      SILTy = i->getType();
+    }
 
-  bindArchetypes(DbgTy.getType());
-  if (IGM.DebugInfo) {
     emitDebugVariableDeclaration(
         addr, DbgTy, SILTy, DS, i->getLoc(), *VarInfo, Indirection,
         AddrDbgInstrKind(i->usesMoveableValueDebugInfo()));
@@ -6511,6 +6541,9 @@ void IRGenSILFunction::visitAllocStackInst(swift::AllocStackInst *i) {
   dbgname = getVarName(i, IsAnonymous);
 # endif
 
+  DebugTypeInfo DbgTy;
+  emitDebugInfoBeforeAllocStack(i, type, DbgTy);
+
   auto stackAddr = type.allocateStack(*this, i->getElementType(), dbgname);
   setLoweredStackAddress(i, stackAddr);
   Address addr = stackAddr.getAddress();
@@ -6525,7 +6558,7 @@ void IRGenSILFunction::visitAllocStackInst(swift::AllocStackInst *i) {
         Ty->getStructOrBoundGenericStruct())
       zeroInit(dyn_cast<llvm::AllocaInst>(addr.getAddress()));
   }
-  emitDebugInfoForAllocStack(i, type, addr.getAddress());
+  emitDebugInfoAfterAllocStack(i, type, DbgTy, addr.getAddress());
 }
 
 void IRGenSILFunction::visitAllocPackInst(swift::AllocPackInst *i) {

test/IRGen/pack_metadata_dealloc.swift

@@ -0,0 +1,21 @@
+// RUN: %target-swift-frontend -emit-ir %s | %FileCheck %s
+
+protocol P {
+  associatedtype A
+  var a: A { get }
+}
+
+func f<each T: P>(_ t: repeat each T) -> (repeat (each T).A) {
+  let data = (repeat (each t).a)
+  return data
+}
+
+// CHECK-LABEL: define hidden swiftcc void @"$s21pack_metadata_dealloc1fy1AQzxQp_txxQpRvzAA1PRzlF"(ptr noalias sret(%swift.opaque) %0, ptr noalias %1, i64 %2, ptr %"each T", ptr %"each T.P") #0 {
+// CHECK: [[SPSAVE1:%.*]] = call ptr @llvm.stacksave.p0()
+// CHECK: [[SPSAVE2:%.*]] = call ptr @llvm.stacksave.p0()
+// CHECK: [[SPSAVE3:%.*]] = call ptr @llvm.stacksave.p0()
+// CHECK-NOT: call ptr llvm.stacksave.p0()
+// CHECK:  call void @llvm.stackrestore.p0(ptr [[SPSAVE3]])
+// CHECK:  call void @llvm.stackrestore.p0(ptr [[SPSAVE2]])
+// CHECK:  call void @llvm.stackrestore.p0(ptr [[SPSAVE1]])
+// CHECK:  ret void