A series of FunC improvements (#378)

* Don't use IFJMP/IFNOTJMP in inline functions * Fix incorrect ifelse branch code generation #374 * Make generate_code_all clearer * Don't replace IFJMP with IF in inner blocks in inline functions * Allow unbalance if/else by using RETALT * Fix wrong PUSHCONT * Bugfix in IF code generation for inline functions * Fix unbalanced if/else * Bugfix and improvements in code generation * Fix analyzing while(0) in func #377 * FunC and Asm.fif: Fix inlining large functions #375 Co-authored-by: SpyCheese <[email protected]>
ton-blockchain · Aug 4, 2022 · 40cec56 · 40cec56
1 parent fecf760
commit 40cec56
Show file tree

Hide file tree

Showing 5 changed files with 138 additions and 75 deletions.
diff --git a/crypto/fift/lib/Asm.fif b/crypto/fift/lib/Asm.fif
@@ -1,15 +1,17 @@
 library TVM_Asm
 // simple TVM Assembler
 variable @atend
+variable @was-split
+false @was-split !
 { "not in asm context" abort } @atend !
 { `normal eq? not abort"must be terminated by }>" } : @normal?
 { @atend @ 1 { @atend ! @normal? } does @atend ! } : @pushatend
 { @pushatend <b } : <{
 { @atend @ execute } : @endblk
-{ `normal @endblk } : }>
+{ false @was-split ! `normal @endblk } : }>
 { }> b> } : }>c
 { }>c <s } : }>s
-{ @atend @ 2 { @atend ! rot b> ref, swap @endblk } does @atend ! <b } : @|
+{ @atend @ 2 { true @was-split ! @atend ! rot b> ref, swap @endblk } does @atend ! <b } : @|
 { @atend @ 3 { @atend ! 2swap rot execute } does @atend ! <b } : @doafter<{
 { over brembits <= } : @havebits
 { rot + -rot + swap } : pair+
@@ -1225,6 +1227,10 @@ variable asm-mode  1 asm-mode !
 } : PROGRAM{
 { over sbits < { s>c <b swap ref, b> <s } if } : @adj-long-proc
 { // i s l
+  dup 0< {
+    negate
+    @was-split @ { drop 0 } if
+  } if
   @adj-long-proc over @procdict @ @procdictkeylen
   idict!+ not abort"cannot define procedure, redefined?"
   @procdict !  2 2 @procinfo~!
@@ -1234,6 +1240,7 @@ variable asm-mode  1 asm-mode !
 { @have-procinfo? { 8 8 @procinfo~! } { drop } cond } : @proc-called
 { 1000 @def-proc } : PROC
 { 0 @def-proc } : PROCREF
+{ -1000 @def-proc } : PROCINLINE
 { @procdict @ @procdictkeylen idict@ abort"procedure already defined"
 } : @fail-ifdef
 { u@?+ { swap abort"first bits are not zeroes" } if } : @cut-zeroes
@@ -1243,6 +1250,7 @@ variable asm-mode  1 asm-mode !
 } : @PROC:<{
 { 1000 @PROC:<{ } : PROC:<{
 { 0 @PROC:<{ } : PROCREF:<{
+{ -1000 @PROC:<{ } : PROCINLINE:<{
 { dup @proc-called CALLDICT } dup : CALL : CALLDICT
 { dup @proc-called JMPDICT } dup : JMP : JMPDICT
 { dup @proc-called PREPAREDICT } dup : PREPARE : PREPAREDICT

diff --git a/crypto/func/analyzer.cpp b/crypto/func/analyzer.cpp
@@ -586,7 +586,7 @@ bool prune_unreachable(std::unique_ptr<Op>& ops) {
         // block1 never executed
         op.block0->last().next = std::move(op.next);
         ops = std::move(op.block0);
-        return false;
+        return prune_unreachable(ops);
       } else if (c_var && c_var->always_true()) {
         if (!prune_unreachable(op.block1)) {
           // block1 never returns

diff --git a/crypto/func/codegen.cpp b/crypto/func/codegen.cpp
@@ -277,12 +277,16 @@ bool Op::generate_code_step(Stack& stack) {
   stack.drop_vars_except(var_info);
   stack.opt_show();
   const auto& next_var_info = next->var_info;
+  bool inline_func = stack.mode & Stack::_InlineFunc;
   switch (cl) {
     case _Nop:
     case _Import:
       return true;
     case _Return: {
       stack.enforce_state(left);
+      if (stack.o.retalt_ && (stack.mode & Stack::_NeedRetAlt)) {
+        stack.o << "RETALT";
+      }
       stack.opt_show();
       return false;
     }
@@ -532,81 +536,65 @@ bool Op::generate_code_step(Stack& stack) {
         return true;
       }
       if (!next->noreturn() && (block0->noreturn() != block1->noreturn())) {
-        // simple fix of unbalanced returns in if/else branches
-        // (to be replaced with a finer condition working in loop bodies)
-        throw src::ParseError{where, "`if` and `else` branches should both return or both not return"};
+        stack.o.retalt_ = true;
       }
       var_idx_t x = left[0];
       stack.rearrange_top(x, var_info[x] && var_info[x]->is_last());
       assert(stack[0] == x);
       stack.opt_show();
       stack.s.pop_back();
       stack.modified();
-      if (block1->is_empty()) {
-        // if (left) block0; ...
-        if (block0->noreturn()) {
-          stack.o << "IFJMP:<{";
-          stack.o.indent();
-          Stack stack_copy{stack};
-          block0->generate_code_all(stack_copy);
-          stack.o.undent();
-          stack.o << "}>";
-          return true;
-        }
-        stack.o << "IF:<{";
+      if (inline_func && (block0->noreturn() || block1->noreturn())) {
+        bool is0 = block0->noreturn();
+        Op* block_noreturn = is0 ? block0.get() : block1.get();
+        Op* block_other = is0 ? block1.get() : block0.get();
+        stack.mode &= ~Stack::_InlineFunc;
+        stack.o << (is0 ? "IF:<{" : "IFNOT:<{");
         stack.o.indent();
-        Stack stack_copy{stack}, stack_target{stack};
-        stack_target.disable_output();
-        stack_target.drop_vars_except(next->var_info);
-        block0->generate_code_all(stack_copy);
-        stack_copy.drop_vars_except(var_info);
-        stack_copy.opt_show();
-        if (stack_copy == stack) {
-          stack.o.undent();
-          stack.o << "}>";
-          return true;
-        }
-        // stack_copy.drop_vars_except(next->var_info);
-        stack_copy.enforce_state(stack_target.vars());
-        stack_copy.opt_show();
-        if (stack_copy.vars() == stack.vars()) {
-          stack.o.undent();
-          stack.o << "}>";
-          stack.merge_const(stack_copy);
-          return true;
-        }
+        Stack stack_copy{stack};
+        block_noreturn->generate_code_all(stack_copy);
         stack.o.undent();
         stack.o << "}>ELSE<{";
         stack.o.indent();
-        stack.merge_state(stack_copy);
-        stack.opt_show();
+        block_other->generate_code_all(stack);
+        if (!block_other->noreturn()) {
+          next->generate_code_all(stack);
+        }
         stack.o.undent();
         stack.o << "}>";
-        return true;
+        return false;
       }
-      if (block0->is_empty()) {
+      if (block1->is_empty() || block0->is_empty()) {
+        bool is0 = block1->is_empty();
+        Op* block = is0 ? block0.get() : block1.get();
+        // if (left) block0; ...
         // if (!left) block1; ...
-        if (block1->noreturn()) {
-          stack.o << "IFNOTJMP:<{";
+        if (block->noreturn()) {
+          stack.o << (is0 ? "IFJMP:<{" : "IFNOTJMP:<{");
           stack.o.indent();
           Stack stack_copy{stack};
-          block1->generate_code_all(stack_copy);
+          stack_copy.mode &= ~Stack::_InlineFunc;
+          stack_copy.mode |= next->noreturn() ? 0 : Stack::_NeedRetAlt;
+          block->generate_code_all(stack_copy);
           stack.o.undent();
           stack.o << "}>";
           return true;
         }
-        stack.o << "IFNOT:<{";
+        stack.o << (is0 ? "IF:<{" : "IFNOT:<{");
         stack.o.indent();
         Stack stack_copy{stack}, stack_target{stack};
         stack_target.disable_output();
         stack_target.drop_vars_except(next->var_info);
-        block1->generate_code_all(stack_copy);
+        stack_copy.mode &= ~Stack::_InlineFunc;
+        block->generate_code_all(stack_copy);
         stack_copy.drop_vars_except(var_info);
         stack_copy.opt_show();
-        if (stack_copy.vars() == stack.vars()) {
+        if ((is0 && stack_copy == stack) || (!is0 && stack_copy.vars() == stack.vars())) {
           stack.o.undent();
           stack.o << "}>";
-          stack.merge_const(stack_copy);
+          if (!is0) {
+            stack.merge_const(stack_copy);
+          }
           return true;
         }
         // stack_copy.drop_vars_except(next->var_info);
@@ -627,33 +615,32 @@ bool Op::generate_code_step(Stack& stack) {
         stack.o << "}>";
         return true;
       }
-      if (block0->noreturn()) {
-        stack.o << "IFJMP:<{";
+      if (block0->noreturn() || block1->noreturn()) {
+        bool is0 = block0->noreturn();
+        Op* block_noreturn = is0 ? block0.get() : block1.get();
+        Op* block_other = is0 ? block1.get() : block0.get();
+        stack.o << (is0 ? "IFJMP:<{" : "IFNOTJMP:<{");
         stack.o.indent();
         Stack stack_copy{stack};
-        block0->generate_code_all(stack_copy);
+        stack_copy.mode &= ~Stack::_InlineFunc;
+        stack_copy.mode |= (block_other->noreturn() || next->noreturn()) ? 0 : Stack::_NeedRetAlt;
+        block_noreturn->generate_code_all(stack_copy);
         stack.o.undent();
         stack.o << "}>";
-        return block1->generate_code_all(stack);
-      }
-      if (block1->noreturn()) {
-        stack.o << "IFNOTJMP:<{";
-        stack.o.indent();
-        Stack stack_copy{stack};
-        block1->generate_code_all(stack_copy);
-        stack.o.undent();
-        stack.o << "}>";
-        return block0->generate_code_all(stack);
+        block_other->generate_code_all(stack);
+        return !block_other->noreturn();
       }
       stack.o << "IF:<{";
       stack.o.indent();
       Stack stack_copy{stack};
+      stack_copy.mode &= ~Stack::_InlineFunc;
       block0->generate_code_all(stack_copy);
       stack_copy.drop_vars_except(next->var_info);
       stack_copy.opt_show();
       stack.o.undent();
       stack.o << "}>ELSE<{";
       stack.o.indent();
+      stack.mode &= ~Stack::_InlineFunc;
       block1->generate_code_all(stack);
       stack.merge_state(stack_copy);
       stack.opt_show();
@@ -669,11 +656,16 @@ bool Op::generate_code_step(Stack& stack) {
       stack.opt_show();
       stack.s.pop_back();
       stack.modified();
+      if (block0->noreturn()) {
+        stack.o.retalt_ = true;
+      }
       if (true || !next->is_empty()) {
         stack.o << "REPEAT:<{";
         stack.o.indent();
         stack.forget_const();
         StackLayout layout1 = stack.vars();
+        stack.mode &= ~Stack::_InlineFunc;
+        stack.mode |= Stack::_NeedRetAlt;
         block0->generate_code_all(stack);
         stack.enforce_state(std::move(layout1));
         stack.opt_show();
@@ -693,11 +685,16 @@ bool Op::generate_code_step(Stack& stack) {
     case _Again: {
       stack.drop_vars_except(block0->var_info);
       stack.opt_show();
-      if (!next->is_empty()) {
+      if (block0->noreturn()) {
+        stack.o.retalt_ = true;
+      }
+      if (!next->is_empty() || inline_func) {
         stack.o << "AGAIN:<{";
         stack.o.indent();
         stack.forget_const();
         StackLayout layout1 = stack.vars();
+        stack.mode &= ~Stack::_InlineFunc;
+        stack.mode |= Stack::_NeedRetAlt;
         block0->generate_code_all(stack);
         stack.enforce_state(std::move(layout1));
         stack.opt_show();
@@ -717,11 +714,16 @@ bool Op::generate_code_step(Stack& stack) {
     case _Until: {
       // stack.drop_vars_except(block0->var_info);
       // stack.opt_show();
+      if (block0->noreturn()) {
+        stack.o.retalt_ = true;
+      }
       if (true || !next->is_empty()) {
         stack.o << "UNTIL:<{";
         stack.o.indent();
         stack.forget_const();
         auto layout1 = stack.vars();
+        stack.mode &= ~Stack::_InlineFunc;
+        stack.mode |= Stack::_NeedRetAlt;
         block0->generate_code_all(stack);
         layout1.push_back(left[0]);
         stack.enforce_state(std::move(layout1));
@@ -749,9 +751,14 @@ bool Op::generate_code_step(Stack& stack) {
       stack.opt_show();
       StackLayout layout1 = stack.vars();
       bool next_empty = false && next->is_empty();
+      if (block0->noreturn()) {
+        stack.o.retalt_ = true;
+      }
       stack.o << "WHILE:<{";
       stack.o.indent();
       stack.forget_const();
+      stack.mode &= ~Stack::_InlineFunc;
+      stack.mode |= Stack::_NeedRetAlt;
       block0->generate_code_all(stack);
       stack.rearrange_top(x, !next->var_info[x] && !block1->var_info[x]);
       stack.opt_show();
@@ -781,11 +788,12 @@ bool Op::generate_code_step(Stack& stack) {
   }
 }
 
-bool Op::generate_code_all(Stack& stack) {
-  if (generate_code_step(stack) && next) {
-    return next->generate_code_all(stack);
-  } else {
-    return false;
+void Op::generate_code_all(Stack& stack) {
+  int saved_mode = stack.mode;
+  auto cont = generate_code_step(stack);
+  stack.mode = (stack.mode & ~Stack::_ModeSave) | (saved_mode & Stack::_ModeSave);
+  if (cont && next) {
+    next->generate_code_all(stack);
   }
 }
 
@@ -796,6 +804,7 @@ void CodeBlob::generate_code(AsmOpList& out, int mode) {
     stack.push_new_var(x);
   }
   ops->generate_code_all(stack);
+  stack.apply_wrappers();
   if (!(mode & Stack::_DisableOpt)) {
     optimize_code(out);
   }

diff --git a/crypto/func/func.cpp b/crypto/func/func.cpp
@@ -99,12 +99,28 @@ void generate_output_func(SymDef* func_sym) {
     if (verbosity >= 2) {
       std::cerr << "\n---------- resulting code for " << name << " -------------\n";
     }
+    bool inline_func = (func_val->flags & 1);
     bool inline_ref = (func_val->flags & 2);
-    *outs << std::string(indent * 2, ' ') << name << " PROC" << (inline_ref ? "REF" : "") << ":<{\n";
-    code.generate_code(
-        *outs,
-        (stack_layout_comments ? Stack::_StkCmt | Stack::_CptStkCmt : 0) | (opt_level < 2 ? Stack::_DisableOpt : 0),
-        indent + 1);
+    const char* modifier = "";
+    if (inline_func) {
+      modifier = "INLINE";
+    } else if (inline_ref) {
+      modifier = "REF";
+    }
+    *outs << std::string(indent * 2, ' ') << name << " PROC" << modifier << ":<{\n";
+    int mode = 0;
+    if (stack_layout_comments) {
+      mode |= Stack::_StkCmt | Stack::_CptStkCmt;
+    }
+    if (opt_level < 2) {
+      mode |= Stack::_DisableOpt;
+    }
+    auto fv = dynamic_cast<const SymValCodeFunc*>(func_sym->value);
+    // Flags: 1 - inline, 2 - inline_ref
+    if (fv && (fv->flags & 1) && code.ops->noreturn()) {
+      mode |= Stack::_InlineFunc;
+    }
+    code.generate_code(*outs, mode, indent + 1);
     *outs << std::string(indent * 2, ' ') << "}>\n";
     if (verbosity >= 2) {
       std::cerr << "--------------\n";