Fix loop RC issues

src/compiler.ts

@@ -1909,12 +1909,25 @@ export class Compiler extends DiagnosticEmitter {
     var outerFlow = this.currentFlow;
     var label = outerFlow.pushBreakLabel();
     var innerFlow = outerFlow.fork();
-    this.currentFlow = innerFlow;
     var breakLabel = "break|" + label;
     innerFlow.breakLabel = breakLabel;
     var continueLabel = "continue|" + label;
     innerFlow.continueLabel = continueLabel;
 
+    // Compile the condition before the body in order to...
+    var condFlow = outerFlow.fork();
+    this.currentFlow = condFlow;
+    var condExpr = module.precomputeExpression(
+      this.makeIsTrueish(
+        this.compileExpression(statement.condition, Type.i32),
+        this.currentType
+      )
+    );
+    assert(!condFlow.hasScopedLocals);
+    // ...unify local states before and after the condition has been executed the first time
+    innerFlow.unifyLocalFlags(condFlow);
+    this.currentFlow = innerFlow;
+
     var stmts = new Array<ExpressionRef>();
     if (statement.statement.kind == NodeKind.BLOCK) {
       this.compileStatements((<BlockStatement>statement.statement).statements, false, stmts);
@@ -1923,12 +1936,6 @@ export class Compiler extends DiagnosticEmitter {
         this.compileStatement(statement.statement)
       );
     }
-    var condExpr = module.precomputeExpression(
-      this.makeIsTrueish(
-        this.compileExpression(statement.condition, Type.i32),
-        this.currentType
-      )
-    );
     var alwaysFalse = false;
     if (getExpressionId(condExpr) == ExpressionId.Const) {
       assert(getExpressionType(condExpr) == NativeType.I32);
@@ -1946,8 +1953,11 @@ export class Compiler extends DiagnosticEmitter {
     // )
     var fallsThrough = !terminates && !innerFlow.is(FlowFlags.BREAKS);
 
-    if (fallsThrough && !alwaysFalse) { // (4)
-      stmts.push(module.br(continueLabel, condExpr));
+    if (fallsThrough) {
+      this.performAutoreleases(innerFlow, stmts);
+      if (!alwaysFalse) { // (4)
+        stmts.push(module.br(continueLabel, condExpr));
+      }
     }
     var expr = flatten(module, stmts, NativeType.None);
     if (fallsThrough && !alwaysFalse || continues) { // (2)
@@ -1958,7 +1968,6 @@ export class Compiler extends DiagnosticEmitter {
     }
 
     // Switch back to the parent flow
-    if (!terminates) this.performAutoreleases(innerFlow, stmts);
     innerFlow.freeScopedLocals();
     outerFlow.popBreakLabel();
     innerFlow.unset(
@@ -2030,16 +2039,26 @@ export class Compiler extends DiagnosticEmitter {
     }
     innerFlow.inheritNonnullIfTrue(condExpr);
 
-    // Compile incrementor
+    // Compile the incrementor before the body in order to...
     var incrementor = statement.incrementor;
     var incrExpr: ExpressionRef = 0;
-    if (incrementor) incrExpr = this.compileExpression(incrementor, Type.void, Constraints.CONV_IMPLICIT | Constraints.WILL_DROP);
+    if (incrementor) {
+      let incrFlow = innerFlow.fork();
+      this.currentFlow = incrFlow;
+      incrExpr = this.compileExpression(incrementor, Type.void, Constraints.CONV_IMPLICIT | Constraints.WILL_DROP);
+      assert(!incrFlow.hasScopedLocals);
+      this.currentFlow = innerFlow;
+      // ...unify local states before and after the incrementor has been executed the first time
+      innerFlow.unifyLocalFlags(incrFlow);
+    }
 
     // Compile body (break: drop out, continue: fall through to incrementor, + loop)
-    var breakLabel = innerFlow.breakLabel = "break|" + label; innerFlow.breakLabel = breakLabel;
-    innerFlow.breakLabel = breakLabel;
+    var bodyFlow = innerFlow.fork();
+    this.currentFlow = bodyFlow;
+    var breakLabel = innerFlow.breakLabel = "break|" + label; bodyFlow.breakLabel = breakLabel;
+    bodyFlow.breakLabel = breakLabel;
     var continueLabel = "continue|" + label;
-    innerFlow.continueLabel = continueLabel;
+    bodyFlow.continueLabel = continueLabel;
     var loopLabel = "loop|" + label;
     var bodyStatement = statement.statement;
     var stmts = new Array<ExpressionRef>();
@@ -2048,9 +2067,16 @@ export class Compiler extends DiagnosticEmitter {
     } else {
       stmts.push(this.compileStatement(bodyStatement));
     }
-    var terminates = innerFlow.is(FlowFlags.TERMINATES);
-    var continues = innerFlow.isAny(FlowFlags.CONTINUES | FlowFlags.CONDITIONALLY_CONTINUES);
-    var breaks = innerFlow.isAny(FlowFlags.BREAKS | FlowFlags.CONDITIONALLY_BREAKS);
+    var terminates = bodyFlow.is(FlowFlags.TERMINATES);
+    var continues = bodyFlow.isAny(FlowFlags.CONTINUES | FlowFlags.CONDITIONALLY_CONTINUES);
+    var breaks = bodyFlow.isAny(FlowFlags.BREAKS | FlowFlags.CONDITIONALLY_BREAKS);
+    var fallsThrough = !terminates && !innerFlow.is(FlowFlags.BREAKS);
+
+    // Finalize body flow
+    if (fallsThrough) this.performAutoreleases(bodyFlow, stmts);
+    bodyFlow.freeScopedLocals();
+    innerFlow.inherit(bodyFlow);
+    this.currentFlow = innerFlow;
 
     // (block $break          ;; (1) skip label (needed anyway) if skipping (4) + no breaks
     //  (initializer)         ;; (2) [may be empty]
@@ -2063,7 +2089,6 @@ export class Compiler extends DiagnosticEmitter {
     //   (br $loop)           ;; (8) skip if skipping (3)
     //  )
     // )
-    var fallsThrough = !terminates && !innerFlow.is(FlowFlags.BREAKS);
     var needsLabel = !alwaysTrue || breaks;
 
     var loop = new Array<ExpressionRef>();

src/flow.ts

@@ -440,6 +440,18 @@ export class Flow {
     return scopedAlias;
   }
 
+  /** Tests if this flow has any scoped locals that must be free'd. */
+  get hasScopedLocals(): bool {
+    if (this.scopedLocals) {
+      for (let scopedLocal of this.scopedLocals.values()) {
+        if (scopedLocal.is(CommonFlags.SCOPED)) { // otherwise an alias
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
   /** Frees this flow's scoped variables and returns its parent flow. */
   freeScopedLocals(): void {
     if (this.scopedLocals) {
@@ -592,6 +604,25 @@ export class Flow {
     this.localFlags = combinedFlags;
   }
 
+  /** Unifies local flags between this and the other flow. */
+  unifyLocalFlags(other: Flow): void {
+    var numThisLocalFlags = this.localFlags.length;
+    var numOtherLocalFlags = other.localFlags.length;
+    for (let i = 0, k = min<i32>(numThisLocalFlags, numOtherLocalFlags); i < k; ++i) {
+      if (this.isLocalFlag(i, LocalFlags.WRAPPED) != other.isLocalFlag(i, LocalFlags.WRAPPED)) {
+        this.unsetLocalFlag(i, LocalFlags.WRAPPED); // assume not wrapped
+      }
+      if (this.isLocalFlag(i, LocalFlags.NONNULL) != other.isLocalFlag(i, LocalFlags.NONNULL)) {
+        this.unsetLocalFlag(i, LocalFlags.NONNULL); // assume possibly null
+      }
+      assert(
+        // having different retain states would be a problem because the compiler
+        // either can't release a retained local or would release a non-retained local
+        this.isAnyLocalFlag(i, LocalFlags.ANY_RETAINED) == other.isAnyLocalFlag(i, LocalFlags.ANY_RETAINED)
+      );
+    }
+  }
+
   /** Checks if an expression of the specified type is known to be non-null, even if the type might be nullable. */
   isNonnull(expr: ExpressionRef, type: Type): bool {
     if (!type.is(TypeFlags.NULLABLE)) return true;

tests/compiler/loop-wrap.json

@@ -0,0 +1,5 @@
+{
+  "asc_flags": [
+    "--runtime none"
+  ]
+}

tests/compiler/loop-wrap.optimized.wat

@@ -0,0 +1,66 @@
+(module
+ (type $FUNCSIG$v (func))
+ (type $FUNCSIG$vi (func (param i32)))
+ (memory $0 0)
+ (export "memory" (memory $0))
+ (export "testAlwaysWrapped" (func $loop-wrap/testAlwaysWrapped))
+ (export "testFirstWrapped" (func $loop-wrap/testFirstWrapped))
+ (export "testSubsequentWrapped" (func $loop-wrap/testSubsequentWrapped))
+ (func $loop-wrap/testAlwaysWrapped (; 0 ;) (type $FUNCSIG$v)
+  (local $0 i32)
+  loop $continue|0
+   local.get $0
+   i32.const 10
+   i32.ne
+   if
+    local.get $0
+    i32.const 1
+    i32.add
+    i32.const 255
+    i32.and
+    local.tee $0
+    br_if $continue|0
+   end
+  end
+ )
+ (func $loop-wrap/testFirstWrapped (; 1 ;) (type $FUNCSIG$v)
+  (local $0 i32)
+  loop $continue|0
+   local.get $0
+   i32.const 255
+   i32.and
+   i32.const 10
+   i32.ne
+   if
+    local.get $0
+    i32.const 1
+    i32.add
+    local.tee $0
+    i32.const 255
+    i32.and
+    br_if $continue|0
+   end
+  end
+ )
+ (func $loop-wrap/testSubsequentWrapped (; 2 ;) (type $FUNCSIG$vi) (param $0 i32)
+  loop $continue|0
+   local.get $0
+   i32.const 255
+   i32.and
+   i32.const 10
+   i32.ne
+   if
+    local.get $0
+    i32.const 1
+    i32.add
+    i32.const 255
+    i32.and
+    local.tee $0
+    br_if $continue|0
+   end
+  end
+ )
+ (func $null (; 3 ;) (type $FUNCSIG$v)
+  nop
+ )
+)

tests/compiler/loop-wrap.ts

@@ -0,0 +1,20 @@
+export function testAlwaysWrapped(): void {
+  var i: u8 = 0; // <--
+  do {
+    if (i == 10) break; // no need to wrap
+  } while (i = (i + 1) & 0xff); // <--
+}
+
+export function testFirstWrapped(): void {
+  var i: u8 = 0;
+  do {
+    if (i == 10) break; // must wrap
+  } while (++i); // <--
+}
+
+export function testSubsequentWrapped(a: i32): void {
+  var i = <u8>a; // <--
+  do {
+    if (i == 10) break; // must wrap
+  } while (i = (i + 1) & 0xff);
+}

tests/compiler/loop-wrap.untouched.wat

@@ -0,0 +1,83 @@
+(module
+ (type $FUNCSIG$v (func))
+ (type $FUNCSIG$vi (func (param i32)))
+ (memory $0 0)
+ (table $0 1 funcref)
+ (elem (i32.const 0) $null)
+ (export "memory" (memory $0))
+ (export "testAlwaysWrapped" (func $loop-wrap/testAlwaysWrapped))
+ (export "testFirstWrapped" (func $loop-wrap/testFirstWrapped))
+ (export "testSubsequentWrapped" (func $loop-wrap/testSubsequentWrapped))
+ (func $loop-wrap/testAlwaysWrapped (; 0 ;) (type $FUNCSIG$v)
+  (local $0 i32)
+  i32.const 0
+  local.set $0
+  block $break|0
+   loop $continue|0
+    local.get $0
+    i32.const 10
+    i32.eq
+    if
+     br $break|0
+    end
+    local.get $0
+    i32.const 1
+    i32.add
+    i32.const 255
+    i32.and
+    local.tee $0
+    br_if $continue|0
+   end
+  end
+ )
+ (func $loop-wrap/testFirstWrapped (; 1 ;) (type $FUNCSIG$v)
+  (local $0 i32)
+  i32.const 0
+  local.set $0
+  block $break|0
+   loop $continue|0
+    local.get $0
+    i32.const 255
+    i32.and
+    i32.const 10
+    i32.eq
+    if
+     br $break|0
+    end
+    local.get $0
+    i32.const 1
+    i32.add
+    local.tee $0
+    i32.const 255
+    i32.and
+    br_if $continue|0
+   end
+  end
+ )
+ (func $loop-wrap/testSubsequentWrapped (; 2 ;) (type $FUNCSIG$vi) (param $0 i32)
+  (local $1 i32)
+  local.get $0
+  local.set $1
+  block $break|0
+   loop $continue|0
+    local.get $1
+    i32.const 255
+    i32.and
+    i32.const 10
+    i32.eq
+    if
+     br $break|0
+    end
+    local.get $1
+    i32.const 1
+    i32.add
+    i32.const 255
+    i32.and
+    local.tee $1
+    br_if $continue|0
+   end
+  end
+ )
+ (func $null (; 3 ;) (type $FUNCSIG$v)
+ )
+)