feat(compiler)!: Add `--memory-base` flag #1115

peblair · 2022-01-22T12:39:03Z

Closes #1111. The introduction of the attrs result to transl_prim felt a little awkward, but I think it was the cleanest way to slide this into the existing well-formedness checks in the compiler.

Also, I'm open to better names for this PR.

Closes #935 (supersedes it)

…onstant-valued primitives to reify these addresses in the runtime

peblair · 2022-01-22T12:42:29Z

@ospencer This appears to have not caused any snapshot changes. Is that not a bit weird?

ospencer

Nice! And it makes sense that no snapshots changed since the newly generated code should be exactly the same as the old code.

The major thing we need to fix is just that one Binaryen setting so it doesn't trash data in the space that you "reserved" 😂

A couple of tests would be good as well. The big one I'm interested in is starting with a memory base that's larger than the initial number of memory pages—I think we start with something like 64 pages. Just want to make sure that if you try to reserve more space than that everything still works.

Also, I believe you need to declare the new flag in the CLI so it can be used.

For the PR title, I think it's not bad really, but we do want to mark it as breaking in case anyone was depending on the static values, since they should use the primitives now. Maybe

feat(compiler)!: Add `--memory-base` flag

to highlight how to use it?

compiler/src/typed/translprim.re

compiler/src/utils/config.re

phated

One style question, but I was also wondering if the attributes are even needed if we land #1074?

compiler/src/codegen/compcore.re

peblair · 2022-01-22T19:15:54Z

@phated

but I was also wondering if the attributes are even needed if we land #1074?

No, they are not. If this lands before that does, then that PR should probably remove the attrs value added here. I'm remiss to say that this should be blocked by #1074, though, since this is such a small change (ATM).

ospencer · 2022-01-22T19:21:27Z

Hmm, we might still need the attributes, since the way we have it working right now would require @unsafe instead of @disableGC. We could maybe change that though.

peblair · 2022-01-22T19:22:25Z

That's a good point. Even less reason for this to be blocked 🙂

peblair · 2022-01-22T19:55:12Z

Tests added.
To be super safe, I checked that the constant gets wired up correctly by veryLarge.gr in the runtime/string WAT, and it looks like it is working great:

compiler/src/codegen/compcore.re

compiler/test/suites/memory_base.re

Co-authored-by: Oscar Spencer <oscar@grain-lang.org>

ospencer

Perfect!

compiler/src/codegen/compcore.re

…lues

phated · 2022-01-22T20:43:27Z

Can you add Closes #935 (supersedes it) to your PR body or link it in the sidebar?

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>

phated

Excellent work! Love that we're knocking 0.5 things down so quickly ❤️

marcusroberts

Looks very good!

jozanza · 2022-01-22T21:29:56Z

compiler/src/typed/translprim.re

@@ -1406,25 +1422,80 @@ let transl_prim = (env, desc) => {
    | Not_found => failwith("This primitive does not exist.")
    };

-  let diable_gc = [(Location.mknoloc("disableGC"), [])];
+  let disable_gc = [(Location.mknoloc("disableGC"), [])];


feat(compiler): Allow base of heap to be configurable and introduce c…

458a1c9

…onstant-valued primitives to reify these addresses in the runtime

peblair added the compiler label Jan 22, 2022

peblair requested a review from a team January 22, 2022 12:39

peblair assigned ospencer Jan 22, 2022

ospencer requested changes Jan 22, 2022

View reviewed changes

ospencer assigned peblair and unassigned ospencer Jan 22, 2022

phated requested changes Jan 22, 2022

View reviewed changes

compiler/src/codegen/compcore.re Outdated Show resolved Hide resolved

PR review comments

c83e803

peblair requested a review from ospencer January 22, 2022 18:59

peblair changed the title ~~feat(compiler): Adjustable heap base~~ feat(compiler)!: Add --memory-base flag Jan 22, 2022

peblair added 4 commits January 22, 2022 20:03

Declare --memory-base flag in CLI

b7b0aee

Change WasmI32 names to more generic ones

f49c451

Fix formatting

3b02345

Fix typo

ec044f0

peblair added 2 commits January 22, 2022 20:35

Disable low_memory_unused when --memory-base is specified

66b7d7d

Add tests for --memory-base

cd10570

peblair requested a review from phated January 22, 2022 19:55

ospencer reviewed Jan 22, 2022

View reviewed changes

compiler/src/codegen/compcore.re Outdated Show resolved Hide resolved

compiler/test/suites/memory_base.re Outdated Show resolved Hide resolved

compiler/test/suites/memory_base.re Outdated Show resolved Hide resolved

peblair and others added 2 commits January 22, 2022 21:11

Update compiler/test/suites/memory_base.re

7544b74

Co-authored-by: Oscar Spencer <oscar@grain-lang.org>

Address PR comments

74ffd7e

peblair requested a review from ospencer January 22, 2022 20:13

ospencer approved these changes Jan 22, 2022

View reviewed changes

phated reviewed Jan 22, 2022

View reviewed changes

compiler/src/codegen/compcore.re Outdated Show resolved Hide resolved

Make malloc use new primitives and add test which checks primitive va…

fe06a0d

…lues

Fix assert test

76610e1

ospencer approved these changes Jan 22, 2022

View reviewed changes

peblair and others added 3 commits January 22, 2022 21:58

Fix GC leak tests

ec2b756

Update compiler/src/codegen/compcore.re

ee4c0ec

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>

Fix mallocTight test

8e7d550

ospencer approved these changes Jan 22, 2022

View reviewed changes

phated approved these changes Jan 22, 2022

View reviewed changes

marcusroberts approved these changes Jan 22, 2022

View reviewed changes

jozanza approved these changes Jan 22, 2022

View reviewed changes

peblair merged commit 0680826 into main Jan 22, 2022

peblair deleted the philip/adjustable-heap-base branch January 22, 2022 22:05

github-actions bot mentioned this pull request Jan 22, 2022

chore: release main #1108

Closed

github-actions bot mentioned this pull request May 16, 2022

chore: release main #1230

Closed

phated mentioned this pull request May 30, 2022

chore: release main phated/grain#1

Merged

github-actions bot mentioned this pull request May 31, 2022

chore: release main #1292

Merged

github-actions bot mentioned this pull request Jul 25, 2022

chore: release main spotandjake/grain#1

Closed

This was referenced Mar 20, 2023

chore: release main alex-snezhko/grain#2

Open

chore: release main alex-snezhko/grain#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compiler)!: Add `--memory-base` flag #1115

feat(compiler)!: Add `--memory-base` flag #1115

peblair commented Jan 22, 2022 •

edited

Loading

peblair commented Jan 22, 2022

ospencer left a comment •

edited

Loading

phated left a comment

peblair commented Jan 22, 2022

ospencer commented Jan 22, 2022

peblair commented Jan 22, 2022

peblair commented Jan 22, 2022

ospencer left a comment

phated commented Jan 22, 2022

phated left a comment

marcusroberts left a comment

jozanza Jan 22, 2022

feat(compiler)!: Add --memory-base flag #1115

feat(compiler)!: Add --memory-base flag #1115

Conversation

peblair commented Jan 22, 2022 • edited Loading

peblair commented Jan 22, 2022

ospencer left a comment • edited Loading

Choose a reason for hiding this comment

phated left a comment

Choose a reason for hiding this comment

peblair commented Jan 22, 2022

ospencer commented Jan 22, 2022

peblair commented Jan 22, 2022

peblair commented Jan 22, 2022

ospencer left a comment

Choose a reason for hiding this comment

phated commented Jan 22, 2022

phated left a comment

Choose a reason for hiding this comment

marcusroberts left a comment

Choose a reason for hiding this comment

jozanza Jan 22, 2022

Choose a reason for hiding this comment

feat(compiler)!: Add `--memory-base` flag #1115

feat(compiler)!: Add `--memory-base` flag #1115

peblair commented Jan 22, 2022 •

edited

Loading

ospencer left a comment •

edited

Loading