AST-58: New Actor IR by nomeata · Pull Request #189 · caffeinelabs/motoko

nomeata · 2019-02-24T22:34:48Z

An actor in the IR is now a bunch of “normal” declarations, follows by a
mapping between exported label and local id.

I dropped the self field from IR.ActorE. The compiler has a hack to
support that particular idiom; I will tackle general actor value capture
next.

The current implementation of ActorE in the backend is most general:
It assumes that the declarations decare some funcref (as they
necessarily do, given their types), stores this in a static locations,
and then exports a small wrapper function that calls this funcref.

In general, that is the right thing to do (the funcref could be captured
from the enviornment). But in most cases, we just want to call the
function directly.

There is also one test case that makes v8 throw a nasty exception, and
it cannot be minimzed separately. Not sure what I am doing wrong. Will
report that against dev.

Also note that a declaration crash throws an internal error, tracked at https://dfinity.atlassian.net/browse/AST-60

An actor in the IR is now a bunch of “normal” declarations, follows by a mapping between exported label and local id. I dropped the `self` field from `IR.ActorE`. The compiler has a hack to support that particular idiom; I will tackle general actor value capture next. The current implementation of `ActorE` in the backend is most general: It assumes that the declarations decare some `funcref` (as they necessarily do, given their types), stores this in a static locations, and then exports a small wrapper function that calls this `funcref`. In general, that is the right thing to do (the funcref could be captured from the enviornment). But in most cases, we just want to call the function directly. There is also one test case that makes v8 throw a nasty exception, and it cannot be minimzed separately. Not sure what I am doing wrong. Will report that against `dev`.

rossberg · 2019-02-25T09:16:37Z

I dropped the self field from IR.ActorE. The compiler has a hack to
support that particular idiom; I will tackle general actor value capture
next.

The current API provides a specific function to get the current actorref, i.e., self. I wonder if we may not need to use that in some cases, e.g., when deploying a standalone actor.

nomeata · 2019-02-25T11:47:18Z

Yes, we have been using that API for a long time, when we statically know that an actorref is actually self. With this change it just becomes a bit more involved to know that.

I would not mind making that explicit in the IR and move this logic out of the backend, though. But that should be a separate change.

nomeata · 2019-02-25T15:56:02Z

(BTW, if you wrote inline comments, you did not submit them yet ;-))

src/compile.ml

src/check_ir.ml

crusso · 2019-02-25T16:20:02Z

src/ir.ml

  | AssertE of exp                             (* assertion *)
  | DeclareE of id * Type.typ * exp            (* local promise *)
  | DefineE of id * mut * exp                  (* promise fulfillment *)
+  | ActorE of dec list * (name * id * Type.typ) list * Type.typ   (* actor *)


Why do you need field types, can't you recover them from the environment and id?

I could; this is for convenience in those code paths that don’t keep a full environment.

I guess a maybe more idiomatic alternative would be to use (name * id) phrase list

crusso · 2019-02-25T16:20:23Z

src/ir.ml

+  | ActorE of dec list * (name * id * Type.typ) list * Type.typ   (* actor *)
  | NewObjE of                                 (* make an object, preserving mutable identity *)
-      Type.obj_sort * (name * id) list * Type.typ
+      Type.obj_sort * (name * id * Type.typ) list * Type.typ


ditto (Why do you need field types, can't you recover them from the environment and id?)

Here I just add them for symmetry with ActorE.

crusso · 2019-02-25T16:24:38Z

test/run-dfinity/ok/flatten-awaitables.dvm-run.ok

-higher-order
-,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
+#
+# Fatal error in v8::ToLocalChecked


what's going on here?

I don’t know, but I reported it as https://dfinity.atlassian.net/browse/M1-513

to deal with varying output of “Illegal Instruction”.

AST-58 A test in preparation of #189

nomeata · 2019-02-27T22:57:18Z

I will have to rebase this on top of #196 once that is in.

nomeata · 2019-02-28T22:24:58Z

I re-did this in #201 (too much else has changed in between).

## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@406decfa...9152a0ff](dfinity/ic-hs@406decf...9152a0f) * [`d4db8d07`](dfinity/ic-hs@d4db8d0) bump nixpkgs to 7c786944f801745310578d1cfc019923396f830c ([dfinity/ic-hs⁠#163](https://github.com/dfinity/ic-hs/issues/163)) * [`31d535d2`](dfinity/ic-hs@31d535d) increase the number of allowed delegations in a request from 4 to 20 ([dfinity/ic-hs⁠#166](https://github.com/dfinity/ic-hs/issues/166)) * [`4a310c0d`](dfinity/ic-hs@4a310c0) support ic0.is_controller ([dfinity/ic-hs⁠#169](https://github.com/dfinity/ic-hs/issues/169)) * [`5fc27bdc`](dfinity/ic-hs@5fc27bd) do not include keep-alive header in httpbin response ([dfinity/ic-hs⁠#170](https://github.com/dfinity/ic-hs/issues/170)) * [`96448083`](dfinity/ic-hs@9644808) drop nix-build-uncached ([dfinity/ic-hs⁠#175](https://github.com/dfinity/ic-hs/issues/175)) * [`c6fbe1f7`](dfinity/ic-hs@c6fbe1f) increase ingress_expiry in reference test suite ([dfinity/ic-hs⁠#176](https://github.com/dfinity/ic-hs/issues/176)) * [`a1b3f670`](dfinity/ic-hs@a1b3f67) add Connection: close header to httpbin ([dfinity/ic-hs⁠#178](https://github.com/dfinity/ic-hs/issues/178)) * [`d3812ffc`](dfinity/ic-hs@d3812ff) increase delegation expiry in tests ([dfinity/ic-hs⁠#182](https://github.com/dfinity/ic-hs/issues/182)) * [`40a46e2f`](dfinity/ic-hs@40a46e2) sync universal-canister with IC repo ([dfinity/ic-hs⁠#177](https://github.com/dfinity/ic-hs/issues/177)) * [`7a6259c2`](dfinity/ic-hs@7a6259c) decrease number of threads and request submission latency ([dfinity/ic-hs⁠#179](https://github.com/dfinity/ic-hs/issues/179)) * [`a9f73dba`](dfinity/ic-hs@a9f73db) fix decoding compressed WASM modules during snapshotting ([dfinity/ic-hs⁠#184](https://github.com/dfinity/ic-hs/issues/184)) * [`64c19a95`](dfinity/ic-hs@64c19a9) bump nixpkgs to eaf03591711b46d21abc7082a8ebee4681f9dbeb ([dfinity/ic-hs⁠#189](https://github.com/dfinity/ic-hs/issues/189)) * [`9152a0ff`](dfinity/ic-hs@9152a0f) add date header to httpbin responses and make http header names lower-case ([dfinity/ic-hs⁠#188](https://github.com/dfinity/ic-hs/issues/188)) Includes and closes #3915. Reason: `ic-hs` and `nixpkgs` must be in sync, so that the artefact caching can work.

AST-58 A test in preparation of this

8725530

Also note that a declaration crash throws an internal error, tracked at https://dfinity.atlassian.net/browse/AST-60

nomeata mentioned this pull request Feb 24, 2019

AST-27 Desugar objects and functions into let #188

Merged

nomeata changed the title ~~AST58: New Actor IR~~ AST-58: New Actor IR Feb 25, 2019

nomeata force-pushed the joachim/AST58 branch from e4c9e66 to 5d1af54 Compare February 25, 2019 01:14

nomeata added 2 commits February 24, 2019 21:30

Normalize output _before_ creating .diff-ir file

8e52578

nomeata force-pushed the joachim/AST58 branch from 5d1af54 to d617ac9 Compare February 25, 2019 01:33

crusso self-requested a review February 25, 2019 16:01

crusso reviewed Feb 25, 2019

View reviewed changes

src/compile.ml Outdated Show resolved Hide resolved

crusso reviewed Feb 25, 2019

View reviewed changes

src/compile.ml Outdated Show resolved Hide resolved

crusso reviewed Feb 25, 2019

View reviewed changes

src/check_ir.ml Show resolved Hide resolved

crusso reviewed Feb 25, 2019

View reviewed changes

nomeata added 2 commits February 25, 2019 20:10

Typos found by Claudio

8239d75

Normalize test suite output more

4d0677c

to deal with varying output of “Illegal Instruction”.

nomeata added a commit that referenced this pull request Feb 27, 2019

Merge pull request #187 from dfinity-lab/joachim/more-tests

2ab6ef1

AST-58 A test in preparation of #189

nomeata mentioned this pull request Feb 28, 2019

New IR.ActorE AST-58 #201

Merged

nomeata closed this Feb 28, 2019

nomeata deleted the joachim/AST58 branch February 28, 2019 22:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AST-58: New Actor IR#189

AST-58: New Actor IR#189
nomeata wants to merge 5 commits intomasterfrom
joachim/AST58

nomeata commented Feb 24, 2019

Uh oh!

rossberg commented Feb 25, 2019

Uh oh!

nomeata commented Feb 25, 2019

Uh oh!

nomeata commented Feb 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crusso Feb 25, 2019

Uh oh!

nomeata Feb 25, 2019

Uh oh!

crusso Feb 25, 2019

Uh oh!

nomeata Feb 25, 2019

Uh oh!

crusso Feb 25, 2019

Uh oh!

nomeata Feb 25, 2019

Uh oh!

nomeata commented Feb 27, 2019

Uh oh!

nomeata commented Feb 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nomeata commented Feb 24, 2019

Uh oh!

rossberg commented Feb 25, 2019

Uh oh!

nomeata commented Feb 25, 2019

Uh oh!

nomeata commented Feb 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crusso Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

nomeata Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

crusso Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

nomeata Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

crusso Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

nomeata Feb 25, 2019

Choose a reason for hiding this comment

Uh oh!

nomeata commented Feb 27, 2019

Uh oh!

nomeata commented Feb 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants