refactor: add optimization phase #1047

jeshecdom · 2024-11-15T11:04:20Z

Issue

Closes #1449.
Closes #970. (As a byproduct, a test was added).

This PR adds an optimization phase in between the typechecking and FunC code generation phases. Currently, the optimization phase only carries out expression simplification by running the interpreter over all expressions in the AST (later, this will be replaced by the partial evaluator). I did this in preparation for other optimization techniques that will be added in the future; for example, constant propagation will be moved to the optimization phase later.

IMPORTANT: This PR removes calls to the interpreter that were previously being carried out during the FunC code generation phase. The FunC generation phase should NOT be calling constant evaluation (this is a job of the optimization phase).

The PR adds two CLI compiler options:

skipTactOptimizationPhase: Default is false. If true, skips the tact optimization phase. Skipping the phase is useful, for example, to carry out tests in which you want to deactivate the interpreter so that you work with the raw program. Simply add the option in your project's json:

"projects": [
    {
      "name": "test",
      "path": "./test.tact",
      "output": "./output",
      "options": { 
          "skipTactOptimizationPhase": true 
      }
    }
  ]

dumpCodeBeforeAndAfterTactOptimizationPhase: Default is false. If true, it will produce two files: one with the program before the optimization phase, and another one after the optimization phase, so that you can make a diff on both files and see what the optimization phase did to the program.

NOTE: This PR does not modify the AST one gets from calling getRawAST on the CompilerContext object ctx. The FunC generation part seems to make use of the functions getAllTypes, getAllStaticFunctions and the like. This PR DOES modify the ASTs produced by those functions. The FunC generator part seems to use only the "sources" from getRawAST.

Checklist

I have updated CHANGELOG.md
I have added tests to demonstrate the contribution is correctly implemented: this usually includes both positive and negative tests, showing the happy path(s) and featuring intentionally broken cases
I have run all the tests locally and no test failure was reported
I have run the linter, formatter and spellchecker
I did not do unrelated and/or undiscussed refactorings

TODOs, like adding the CLI option to activate/deactivate optimization phase.

Added CLI options for skipping optimization phase and for dumping optimized tact code.

Further fixes: - subexpressions inside struct instances did not have their types registered correctly. - makeValueExpression now receives a SrcInfo object.

verytactical · 2024-11-26T21:48:47Z

schemas/configSchema.json

+              "dumpOptimizedTactCode": {
+                "type": "boolean",
+                "default": false,
+                "description": "False by default. If set to true, dumps the code produced by the Tact code optimization phase. In case the optimization phase is skipped, this option is ignored."


"Smooth" (in math sense) behavior would be to dump the code anyway, yet unoptimized. "Dump the optimized code" is just another separate optional compilation step that happens to be in the list after "optimize" step.

verytactical · 2024-11-26T21:49:06Z

src/interpreter.ts

@@ -960,7 +960,7 @@ export class Interpreter {
                if (foundContractConst.value !== undefined) {
                    return foundContractConst.value;
                } else {
-                    throwErrorConstEval(
+                    throwNonFatalErrorConstEval(


Sorry I don't understand this change, and I suspect a future reader wouldn't either.

When do we throw non-fatal errors in general?

When do we throw non-fatal errors in general?

When we want to stop the constant evaluation, but continue compiling the rest of the contract. Like, when not being able to compute something at compile-time with ability to fallback to run-time implementation for that something.

stopEvaluation()?

Yeah, but the current pipeline kinda relies on throwing and catching errors and not on errors as values ¯\_(ツ)_/¯

verytactical · 2024-11-26T21:49:27Z

src/optimizer/optimization_phase.ts

+    return ctx;
+}
+
+export function dump_tact_code(ctx: CompilerContext, file: string) {


It's odd that the function calls prettyPrint several times.

prettyPrint is supposed to print the whole AST node, whatever its type is.

Either we're missing another type of AST node, or this is actually something in the lines of prettyPrint({ kind: 'module', ... }).

verytactical · 2024-11-26T21:49:37Z

src/optimizer/optimization_phase.ts

+        program += `${prettyPrint(t.ast)}\n\n`;
+    }
+
+    writeFileSync(file, program, {


writeFile does w by default, doesn't it?

Also *Sync functions are synchronous and stop the whole JS virtual machine until they're done with what they were doing. It's fine to use a few of these in tests or in build scripts, but I think they can never be used from any of src/**/*.ts.

Eventually we might want to read several files during compilation, and in case of Web IDE probably even download those files over wire. *Sync is impossible to emulate in web (barring some unfortunate deprecated APIs that make the whole browser tab unresponsive), and there'll be no easy way to unSync the code in the future.

This should probably use import fs from "node:fs/promises"

verytactical · 2024-11-26T21:49:54Z

src/optimizer/test/__snapshots__/expr_simplification.spec.ts.snap

@@ -0,0 +1,184 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`expression-simplification should fail expression simplification for interpreter-called-when-no-contract 1`] = `


Very, very wise use of Jest snapshots.

I was thinking to investigate how to do this, but you've already done it. Thanks!

verytactical · 2024-11-26T22:03:43Z