Only inline into active positions #695

b-studios · 2024-11-15T09:58:26Z

No description provided.

Co-authored-by: Jonathan Brachthäuser <[email protected]>

'stream' got slightly refactored (using `Any` everywhere now), fixed a few bugs, added a few examples of push streams, pull streams, their infinite variants, `limit`, `zip`, ... --------- Co-authored-by: Philipp Schuster <[email protected]>

This special flag annoyed me, so here is a version with pointer comparisons instead.

Co-authored-by: Philipp Schuster <[email protected]>

As discussed in #561 (comment), there's a missing rewrite. This fix is necessary for the `map` and `set` examples, as well as some of my AoC solutions using them, so I'd like to upstream it, if possible. Co-authored-by: Marvin <[email protected]>

## Motivation Resolves #723 Expanded the `stdlib` by adding a function on lists which counts how often a given predicate evaluates `true`. The Effekt Working group decided on the name `count` ## Changes Implemented the `count[A](list: List[A]){ predicate: A => Bool }: Int ` function in libraries/common/list.effekt with fitting Documentation including Examples. Implemented a test and check file file in the examples/stdlib/list directory ## Testing I locally ran the `sbt:effekt>test` successfully and tested some examples. It can be tested the same way ## Notes This is my first contribution to the effekt-lang, so I am happy to hear feedback and change things if there's need for it:) - regards Jakub

…mespace (#728) I've been sitting on these changes for a good while now, so let's upstream 'em. Among others adds: - ANSI256 colours - all sorts of combinators for manipulating the terminal: change title, move cursor, clear line, clear screen, hyperlink, ... Be warned, this might break some users who rely on escapes and/or the 'string/tty' module directly.

Currently, it's not possible to import, say, `mutable/map` when working on the `jsWeb` backend. This PR fixes that.

Fixes #724.

This fixes the failing program on effekt plots and fixes #732

Reverts #730 as it seems like an incorrect fix, judging both by the Effekt Working Group meeting, and the graphs after merging it: ![image](https://github.com/user-attachments/assets/67f28d10-ef57-4529-b47d-4df1d342bd63) ![image](https://github.com/user-attachments/assets/e6de9240-d9fd-4aeb-8f45-79a225307811)

Adds a quick regression test for a handler in tail position that uses a mutable variable (minimised reproduction of the underlying issue). The test passes on current `master` after #692. Crashes the compiler on `v0.9.0`, but works on `v0.8.0`.

Resolves #717 I am fully aware that this is not the best way to do things, a larger refactor would be needed (see the TODO in code). However, it's a somewhat pressing issue as we have users trying to work with `js-web` blocked on this... As a bonus, I added a `id=app` div to the generated HTML.

Based on #622. This adds: - Some (minimal) features in the effekt stdlib and the subset of #527 that it depends on - A simple Scanner library (lexing string literals and numbers for now) - A library to parse and print (minified, not pretty) json --------- Co-authored-by: Philipp Schuster <[email protected]>

Update kiama to support LSP notebooks

Fixes #749

…of old ones (#754) See discussion in #733 for how I traced down the bug. Summary: At the point where we dealias the binding `l = m`, the core tree looks like: ``` let l = m; def b_k_1(r, xs) = // note the l here: let v_3 = run {link(k, v, l, r)} go(bitwiseShl225(level, 1), v_3, xs); ... ``` As it can be clearly seen, `l` is used in the join points `b_k_...`. The bindings, after rewriting look like this. ``` let l = m; def b_k_1(r, xs) = // note the m here: let v_3 = run {link(k, v, m, r)} go(bitwiseShl225(level, 1), v_3, xs); ... ``` It appears the renaming was successful. However, the rewritten defs are updated in the `InlineContext`, which can be seen by inspecting `rewrite(List[Definition])` https://github.com/effekt-lang/effekt/blob/9007b059291153c7d9279ec729d62803a7b47275/effekt/shared/src/main/scala/effekt/core/Inline.scala#L88C7-L97 and its callsite: https://github.com/effekt-lang/effekt/blob/9007b059291153c7d9279ec729d62803a7b47275/effekt/shared/src/main/scala/effekt/core/Inline.scala#L124-L126 So, the next time a `def` is inlined, the old `def` is inlined. It still refers to the old, aliased but now removed, binding... Hence causing the error. In this PR, I update the defs in the context to now contain the rewritten (dealiased) right hand sides. --------- Co-authored-by: Jiří Beneš <[email protected]>

b-studios · 2024-12-25T13:33:05Z

This PR is now ready for review, some minor cleanup changes might be pending.

It would be great if we could test it with all the problematic examples that caused the previous inliner to fail, before merging.

…ions

b-studios · 2024-12-25T14:29:04Z

effekt/shared/src/main/scala/effekt/core/optimizer/Deadcode.scala

+    case Reset(body) =>
+      rewrite(body) match {
+        case BlockLit(tparams, cparams, vparams, List(prompt), body) if !reachable.isDefinedAt(prompt.id) => body
+        case b => Stmt.Reset(b)
+      }


Not sure, but we could also move this to a separate phase.

in favor:

deadcode would really only do deadcode elimination

against:

removing an unused reset is a bit like removing a dead binding, so it does fit

a separate phase would potentially cost us some time and conceptual overhead.

b-studios · 2024-12-25T14:36:27Z

effekt/shared/src/main/scala/effekt/core/optimizer/Optimizer.scala

+    }
+
+    // (3) normalize once and remove beta redexes
+    tree = Context.timed("normalize-1", source.name) { normalize(tree) }


@marvinborner will these new timings cause problems with plots?

b-studios · 2024-12-25T14:37:40Z

effekt/shared/src/main/scala/effekt/core/optimizer/StaticArguments.scala

            dropStatic(staticT, targs),
            dropStatic(staticV, vargs).map(rewrite),
            dropStatic(staticB, bargs).map(rewrite))
-        case _ => app(rewrite(b), targs, vargs.map(rewrite), bargs.map(rewrite))
+        case _ => Stmt.App(rewrite(b), targs, vargs.map(rewrite), bargs.map(rewrite))


Here we do not perform beta reduction as this will be done later by Normalizer.

b-studios · 2024-12-25T14:38:39Z

effekt/shared/src/main/scala/effekt/core/optimizer/StaticArguments.scala

@@ -120,22 +123,22 @@ object StaticArguments {

  def rewrite(s: Stmt)(using C: StaticArgumentsContext): Stmt = s match {
    case Stmt.Scope(definitions, body) =>
-      scope(rewrite(definitions), rewrite(body))
+      normal.Scope(rewrite(definitions), rewrite(body))


This regularly is the only smart constructor we use since it is stupid to accidentally create a scope with no bindings.

b-studios · 2024-12-25T14:39:29Z

effekt/shared/src/main/scala/effekt/generator/chez/Transformer.scala

    case Region(body) => chez.Builtin("with-region", toChez(body))

-    case other => chez.Let(Nil, toChez(other))
+    case s: Scope => chez.Let(Nil, toChez(s))


Annotated since otherwise we might be missing cases (which happened) and then loop during code gen.

b-studios · 2024-12-25T14:41:09Z

effekt/shared/src/main/scala/effekt/machine/Transformer.scala

@@ -168,7 +170,7 @@ object Transformer {
        transform(vargs, bargs).run { (values, blocks) =>
          callee match {
            case Block.BlockVar(id, annotatedTpe, annotatedCapt) =>
-              BPC.info.getOrElse(id, sys.error(s"Cannot find block info for ${id}.\n${BPC.info}")) match {
+              BPC.info.getOrElse(id, sys.error(pp"In ${stmt}. Cannot find block info for ${id}: ${annotatedTpe}.\n${BPC.info}")) match {


This shouldn't happen, but it does (sadly quite frequently), so I dump a bit of additional info here.

b-studios · 2024-12-25T15:42:23Z

Before the last (two) commit(s), product_early was much better on LLVM. This might be worth investigating.

b-studios · 2024-12-26T10:24:48Z

Turns out in d44618f I removed some of the normalization functions from core.Tree. In turn, some empty continuation pushes of the form"

val x = STMT
x

were not removed anymore.

Instead of adding this back, 84f1db6 now only removes empty continuations.

The reasoning is that some the previous "normalization" also performed commuting conversions of val x = { val y = stmt1 }; stmt2. However, this extends the lifetime and is not "conservative".

b-studios and others added 30 commits November 14, 2024 18:43

Start micropasses by binding subexpressions

388790e

Add simple normalizer

8e332f3

If no info assume non-recursive

c88cbfe

Very hackily establish Barendregdt while keeping usage information

38e0356

Do not inline side effecting expressions

b6f2c6b

Replace reachability by very strange ad-hoc test

12332c0

Merge branch 'master' into feature/micropasses

299d8dd

Merge branch 'master' into feature/micropasses

3ea9f0e

JS runtime: Extract RESUME function (#708)

606713e

Co-authored-by: Jonathan Brachthäuser <[email protected]>

Bump version to 0.11.0

cd82767

Refactor 'stream', add a few examples (#705)

6f63dde

'stream' got slightly refactored (using `Any` everywhere now), fixed a few bugs, added a few examples of push streams, pull streams, their infinite variants, `limit`, `zip`, ... --------- Co-authored-by: Philipp Schuster <[email protected]>

Bump version to 0.12.0

48a6844

Remove flag from %StackValue (#721)

cee823c

This special flag annoyed me, so here is a version with pointer comparisons instead.

Use noalias on StackValues (#704)

31c30ba

Co-authored-by: Philipp Schuster <[email protected]>

Bump version to 0.13.0

9538871

Allow importing JS-specific Effekt libraries on jsWeb backend (#738)

a6f0cbc

Currently, it's not possible to import, say, `mutable/map` when working on the `jsWeb` backend. This PR fixes that.

Fix lexing of ' (#742)

2f04eb0

Fixes #724.

Make sure unique name is actually fresh (#741)

81e101c

This fixes the failing program on effekt plots and fixes #732

Add a regression check for #692 (#693)

513a87d

Adds a quick regression test for a handler in tail position that uses a mutable variable (minimised reproduction of the underlying issue). The test passes on current `master` after #692. Crashes the compiler on `v0.9.0`, but works on `v0.8.0`.

Bump version to 0.14.0

74183d1

Update/kiama lsp (#753)

f308756

Update kiama to support LSP notebooks

Add 'show' for 'Byte' on JavaScript (#752)

4394f41

Fixes #749

Bump version to 0.15.0

f49a2a6

b-studios added 9 commits December 23, 2024 11:33

Remove tailresumption (requires normalization AFTER deadcode)

2e8f07b

Go back to usage instead of ad hoc cycle detection

968fb97

Don't inline accidentally

38a96a0

Try to improve precision of reachability analysis

e40edb7

Inline functions used once, anyways

93e81a0

Add timing

56e18c3

Clean up a bit an separate out tailresumption optimization

52c1b74

Replace inliner by normalizer

b6050a4

Move optimization related phases to effekt.core.optimizer

d44618f

b-studios changed the title ~~Start micropasses by binding subexpressions~~ Only inline into active positions Dec 25, 2024

b-studios marked this pull request as ready for review December 25, 2024 13:30

Merge branch 'master' into feature/micropasses

c789002

b-studios added 2 commits December 25, 2024 15:21

Comment in JS tests again

aca6d03

Comment in test that is fixed accidentally by inlining unique definit…

bfdb32c

…ions

b-studios commented Dec 25, 2024

View reviewed changes

Clean up optimizer a bit

0655a2d

b-studios commented Dec 25, 2024

View reviewed changes

b-studios added 2 commits December 25, 2024 16:19

Try to update usage information when reducing

cf88bde

Remove usage if it drops to zero

187ba4c

Remove empty continuations

84f1db6

b-studios added 3 commits December 26, 2024 16:14

Expand documentation

6c20394

Make show non-lazy

b4716d1

Remove unnecessary abstraction

0718d60

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only inline into active positions #695

Only inline into active positions #695

b-studios commented Nov 15, 2024

b-studios commented Dec 25, 2024

b-studios Dec 25, 2024

b-studios Dec 25, 2024

b-studios Dec 25, 2024

b-studios Dec 25, 2024

b-studios Dec 25, 2024

b-studios Dec 25, 2024

b-studios commented Dec 25, 2024

b-studios commented Dec 26, 2024

Only inline into active positions #695

Are you sure you want to change the base?

Only inline into active positions #695

Conversation

b-studios commented Nov 15, 2024

b-studios commented Dec 25, 2024

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios Dec 25, 2024

Choose a reason for hiding this comment

b-studios commented Dec 25, 2024

b-studios commented Dec 26, 2024