[silgen] Change foreach loop emission to use the new SwitchEnumBuilder.

lib/SILGen/SILGenStmt.cpp

@@ -11,14 +11,15 @@
 //===----------------------------------------------------------------------===//
 
 #include "SILGen.h"
-#include "Scope.h"
 #include "Condition.h"
 #include "Initialization.h"
 #include "LValue.h"
 #include "RValue.h"
+#include "Scope.h"
+#include "SwitchCaseFullExpr.h"
 #include "swift/AST/AST.h"
-#include "swift/SIL/SILArgument.h"
 #include "swift/AST/DiagnosticsSIL.h"
+#include "swift/SIL/SILArgument.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace swift;
@@ -745,7 +746,7 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) {
   // Emit the 'iterator' variable that we'll be using for iteration.
   LexicalScope OuterForScope(SGF.Cleanups, SGF, CleanupLocation(S));
   SGF.visitPatternBindingDecl(S->getIterator());
-  
+
   // If we ever reach an unreachable point, stop emitting statements.
   // This will need revision if we ever add goto.
   if (!SGF.B.hasValidInsertionPoint()) return;
@@ -756,100 +757,130 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) {
   // will be in the buffer.
   auto optTy = S->getIteratorNext()->getType()->getCanonicalType();
   auto &optTL = SGF.getTypeLowering(optTy);
-  SILValue nextBufOrValue;
+  SILValue addrOnlyBuf;
+  ManagedValue nextBufOrValue;
 
   if (optTL.isAddressOnly())
-    nextBufOrValue = SGF.emitTemporaryAllocation(S, optTL.getLoweredType());
-  
+    addrOnlyBuf = SGF.emitTemporaryAllocation(S, optTL.getLoweredType());
+
   // Create a new basic block and jump into it.
   JumpDest loopDest = createJumpDest(S->getBody());
   SGF.B.emitBlock(loopDest.getBlock(), S);
-  
+
   // Set the destinations for 'break' and 'continue'.
   JumpDest endDest = createJumpDest(S->getBody());
   SGF.BreakContinueDestStack.push_back({ S, endDest, loopDest });
 
+  // Then emit the loop destination block.
+  //
   // Advance the generator.  Use a scope to ensure that any temporary stack
   // allocations in the subexpression are immediately released.
   if (optTL.isAddressOnly()) {
     Scope InnerForScope(SGF.Cleanups, CleanupLocation(S->getIteratorNext()));
-    auto nextInit = SGF.useBufferAsTemporary(nextBufOrValue, optTL);
+    auto nextInit = SGF.useBufferAsTemporary(addrOnlyBuf, optTL);
     SGF.emitExprInto(S->getIteratorNext(), nextInit.get());
-    nextInit->getManagedAddress().forward(SGF);
-  } else {
-    Scope InnerForScope(SGF.Cleanups, CleanupLocation(S->getIteratorNext()));
     nextBufOrValue =
-      SGF.emitRValueAsSingleValue(S->getIteratorNext()).forward(SGF);
-  }
-
-  // Continue if the value is present.
-  Condition Cond = SGF.emitCondition(
-         SGF.emitDoesOptionalHaveValue(S, nextBufOrValue), S,
-         /*hasFalseCode=*/false, /*invertValue=*/false);
-
-  if (Cond.hasTrue()) {
-    Cond.enterTrue(SGF);
-    SGF.emitProfilerIncrement(S->getBody());
-
-    // Emit the loop body.
-    // The declared variable(s) for the current element are destroyed
-    // at the end of each loop iteration.
+        ManagedValue::forUnmanaged(nextInit->getManagedAddress().forward(SGF));
+  } else {
+    // SEMANTIC SIL TODO: I am doing this to match previous behavior. We need to
+    // forward tmp below to ensure that we do not prematurely destroy the
+    // induction variable at the end of scope. I tried to use the
+    // CleanupRestorationScope and dormant, but it seemingly did not work and I
+    // do not have time to look into this now = (.
+    SILValue tmpValue;
+    bool hasCleanup;
     {
-      Scope InnerForScope(SGF.Cleanups, CleanupLocation(S->getBody()));
-      // Emit the initialization for the pattern.  If any of the bound patterns
-      // fail (because this is a 'for case' pattern with a refutable pattern,
-      // the code should jump to the continue block.
-      InitializationPtr initLoopVars
-        = SGF.emitPatternBindingInitialization(S->getPattern(), loopDest);
-      ManagedValue val;
-
-      // If we had a loadable "next" generator value, we know it is present.
-      // Get the value out of the optional, and wrap it up with a cleanup so
-      // that any exits out of this scope properly clean it up.
-      if (optTL.isLoadable()) {
-        val = SGF.emitManagedRValueWithCleanup(nextBufOrValue);
-      } else {
-        val = SGF.emitManagedBufferWithCleanup(nextBufOrValue);
-      }
-      val = SGF.emitUncheckedGetOptionalValueFrom(S, val, optTL,
-                                            SGFContext(initLoopVars.get()));
-      if (!val.isInContext())
-        RValue(SGF, S, optTy.getAnyOptionalObjectType(), val)
-          .forwardInto(SGF, S, initLoopVars.get());
-
-      // Now that the pattern has been initialized, check any where condition.
-      // If it fails, loop around as if 'continue' happened.
-      if (auto *Where = S->getWhere()) {
-        auto cond = SGF.emitCondition(Where, /*hasFalse*/false, /*invert*/true);
-        // If self is null, branch to the epilog.
-        cond.enterTrue(SGF);
-        SGF.Cleanups.emitBranchAndCleanups(loopDest, Where, { });
-        cond.exitTrue(SGF);
-        cond.complete(SGF);
-      }
-
-      visit(S->getBody());
+      Scope InnerForScope(SGF.Cleanups, CleanupLocation(S->getIteratorNext()));
+      ManagedValue tmp = SGF.emitRValueAsSingleValue(S->getIteratorNext());
+      hasCleanup = tmp.hasCleanup();
+      tmpValue = tmp.forward(SGF);
     }
-
-    // Loop back to the header.
-    if (SGF.B.hasValidInsertionPoint()) {
-      // Associate the loop body's closing brace with this branch.
-      RegularLocation L(S->getBody());
-      L.pointToEnd();
-      SGF.B.createBranch(L, loopDest.getBlock());
-    }
-    Cond.exitTrue(SGF);
-  }
-
-  // Complete the conditional execution.
-  Cond.complete(SGF);
-
+    nextBufOrValue = hasCleanup ? SGF.emitManagedRValueWithCleanup(tmpValue)
+                                : ManagedValue::forUnmanaged(tmpValue);
+  }
+
+  SILBasicBlock *failExitingBlock = createBasicBlock();
+  SwitchEnumBuilder switchEnumBuilder(SGF.B, S, nextBufOrValue);
+
+  switchEnumBuilder.addCase(
+      SGF.getASTContext().getOptionalSomeDecl(), createBasicBlock(),
+      loopDest.getBlock(),
+      [&](ManagedValue inputValue, SwitchCaseFullExpr &scope) {
+        SGF.emitProfilerIncrement(S->getBody());
+
+        // Emit the loop body.
+        // The declared variable(s) for the current element are destroyed
+        // at the end of each loop iteration.
+        {
+          Scope innerForScope(SGF.Cleanups, CleanupLocation(S->getBody()));
+          // Emit the initialization for the pattern.  If any of the bound
+          // patterns
+          // fail (because this is a 'for case' pattern with a refutable
+          // pattern,
+          // the code should jump to the continue block.
+          InitializationPtr initLoopVars =
+              SGF.emitPatternBindingInitialization(S->getPattern(), loopDest);
+
+          // If we had a loadable "next" generator value, we know it is present.
+          // Get the value out of the optional, and wrap it up with a cleanup so
+          // that any exits out of this scope properly clean it up.
+          //
+          // *NOTE* If we do not have an address only value, then inputValue is
+          // *already properly unwrapped.
+          if (optTL.isAddressOnly()) {
+            inputValue =
+                SGF.emitManagedBufferWithCleanup(nextBufOrValue.getValue());
+            inputValue = SGF.emitUncheckedGetOptionalValueFrom(
+                S, inputValue, optTL, SGFContext(initLoopVars.get()));
+          }
+
+          if (!inputValue.isInContext())
+            RValue(SGF, S, optTy.getAnyOptionalObjectType(), inputValue)
+                .forwardInto(SGF, S, initLoopVars.get());
+
+          // Now that the pattern has been initialized, check any where
+          // condition.
+          // If it fails, loop around as if 'continue' happened.
+          if (auto *Where = S->getWhere()) {
+            auto cond =
+                SGF.emitCondition(Where, /*hasFalse*/ false, /*invert*/ true);
+            // If self is null, branch to the epilog.
+            cond.enterTrue(SGF);
+            SGF.Cleanups.emitBranchAndCleanups(loopDest, Where, {});
+            cond.exitTrue(SGF);
+            cond.complete(SGF);
+          }
+
+          visit(S->getBody());
+        }
+
+        // If we emitted an unreachable in the body, we will not have a valid
+        // insertion point. Just return early.
+        if (!SGF.B.hasValidInsertionPoint())
+          return;
+
+        // Otherwise, associate the loop body's closing brace with this branch.
+        RegularLocation L(S->getBody());
+        L.pointToEnd();
+        scope.exit(L);
+      });
+
+  // We add loop fail block, just to be defensive about intermediate
+  // transformations performing cleanups at scope.exit(). We still jump to the
+  // contBlock.
+  switchEnumBuilder.addCase(
+      SGF.getASTContext().getOptionalNoneDecl(), createBasicBlock(),
+      failExitingBlock,
+      [&](ManagedValue inputValue, SwitchCaseFullExpr &scope) {
+        assert(!inputValue && "None should not be passed an argument!");
+        scope.exit(S);
+      });
+
+  std::move(switchEnumBuilder).emit();
+
+  SGF.B.emitBlock(failExitingBlock);
   emitOrDeleteBlock(SGF, endDest, S);
   SGF.BreakContinueDestStack.pop_back();
-
-  // We do not need to destroy the value in the 'nextBuf' slot here, because
-  // either the 'for' loop finished naturally and the buffer contains '.None',
-  // or we exited by 'break' and the value in the buffer was consumed.
 }
 
 void StmtEmitter::visitBreakStmt(BreakStmt *S) {

lib/SILGen/SwitchCaseFullExpr.cpp

@@ -23,10 +23,6 @@ SwitchCaseFullExpr::SwitchCaseFullExpr(SILGenFunction &SGF, CleanupLocation loc,
     : SGF(SGF), scope(SGF.Cleanups, loc), loc(loc),
       contBlock(contBlock ? *contBlock : *SGF.B.splitBlockForFallthrough()) {}
 
-SwitchCaseFullExpr::~SwitchCaseFullExpr() {
-  assert(!scope.isValid() && "Switch Case Full Expr was not popped?!");
-}
-
 void SwitchCaseFullExpr::exit(SILLocation loc, ArrayRef<SILValue> branchArgs) {
   assert(SGF.B.hasValidInsertionPoint());
   scope.pop();

lib/SILGen/SwitchCaseFullExpr.h

@@ -36,7 +36,7 @@ class SwitchCaseFullExpr {
   explicit SwitchCaseFullExpr(SILGenFunction &SGF, CleanupLocation loc,
                               SILBasicBlock *contBlock = nullptr);
 
-  ~SwitchCaseFullExpr();
+  ~SwitchCaseFullExpr() = default;
 
   SwitchCaseFullExpr(const SwitchCaseFullExpr &) = delete;
   SwitchCaseFullExpr &operator=(const SwitchCaseFullExpr &) = delete;

test/DebugInfo/dbgvalue-insertpt.swift

@@ -1,9 +1,18 @@
 // RUN: %target-swift-frontend -g -emit-ir %s | %FileCheck %s
 for i in 0 ..< 3 {
   // CHECK: %[[ALLOCA:[0-9]+]] = alloca %TSiSg
+  // CHECK: %i.addr = alloca i{{32|64}}
   // CHECK: %[[CAST:[0-9]+]] = bitcast %TSiSg* %[[ALLOCA]] to i{{32|64}}*
   // CHECK: %[[LD:[0-9]+]] = load i{{32|64}}, i{{32|64}}* %[[CAST]]
-  // CHECK-NEXT: call void @llvm.dbg.value(metadata i{{32|64}} %[[LD]],
+  // CHECK: br i1 {{%.*}}, label %[[FAIL:.*]], label %[[SUCCESS:.*]],
+  //
+  // CHECK: ; <label>:[[SUCCESS]]:
+  // CHECK: br label %[[NEXT_BB:.*]],
+  //
+  // CHECK: ; <label>:[[NEXT_BB]]:
+  // CHECK: %[[PHI_VAL:.*]] = phi i{{32|64}} [ %[[LD]], %[[SUCCESS]] ]
+  // CHECK: store i{{32|64}} %[[PHI_VAL]], i{{32|64}}* %i.addr
+  // CHECK-NEXT: call void @llvm.dbg.declare(metadata i{{32|64}}* %i.addr,
   // CHECK-SAME:                           metadata ![[I:[0-9]+]],
   // CHECK: ![[I]] = !DILocalVariable(name: "i",
 }