Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push/Enter and Constant Propagation for Michelson #305

Merged
merged 41 commits into from Feb 11, 2020

Conversation

@mariari
Copy link
Contributor

mariari commented Jan 14, 2020

No description provided.

@mariari mariari requested a review from cwgoes Jan 14, 2020
@mariari

This comment has been minimized.

Copy link
Contributor Author

mariari commented Jan 15, 2020

This commit will also add a size parameter to the stack, that way we don't have to do length checks all the time.

Constant values will be stored on this stack (inefficient to lookup!), however they won't count towards the length of the stack, and will be noted as a skip when looking up offsets!

@mariari mariari changed the title Some minor changes. Push/Enter and Constant Propagation for Michelson Jan 17, 2020
mariari and others added 2 commits Feb 6, 2020
show endType
]
case maybeInstr of
Left _ pure maybeInstr

This comment has been minimized.

Copy link
@cwgoes

cwgoes Feb 6, 2020

Collaborator

Note that we can still check the stack type here, by simulating what the lambda would be.

src/Juvix/Backends/Michelson/Compilation/Datatypes.hs Outdated Show resolved Hide resolved
src/Juvix/Backends/Michelson/Compilation/Datatypes.hs Outdated Show resolved Hide resolved
-- todo: formalise this a bit
-- todo: deal with michelson builtins which take lambdas
-- they will have to use this function or something
undefined

This comment has been minimized.

Copy link
@cwgoes

cwgoes Feb 6, 2020

Collaborator

Ah here we need to call funcToLambda

This comment has been minimized.

Copy link
@mariari

mariari Feb 7, 2020

Author Contributor

it seems we need to add paramTy also

This comment has been minimized.

Copy link
@cwgoes

cwgoes Feb 9, 2020

Collaborator

indeed we do

$ filter (VStack.inT . fst)
$ take (length xs) currentStack

realValues = realValuesGen captures

This comment has been minimized.

Copy link
@cwgoes

cwgoes Feb 6, 2020

Collaborator

I wonder if we should alter the formatter, I don't really like the newlines here

Left (LamPartial ops parCaptures parArgs parBody parTy) do
-- todo: are these captures correct
-- Will this type be correct if we compile this to a lambda?
recurseApplication (parCaptures, parArgs, parBody) parTy extraApplied (insts <> ops) paramTy

This comment has been minimized.

Copy link
@cwgoes

cwgoes Feb 6, 2020

Collaborator

Will we correctly drop the initially evaluated arguments (insts) here (later on)? I'm not sure that we will

This comment has been minimized.

Copy link
@mariari

mariari Feb 7, 2020

Author Contributor

I don't see where we drop any of this, correct

src/Juvix/Backends/Michelson/Compilation/Term.hs Outdated Show resolved Hide resolved
test/Backends/Michelson.hs Outdated Show resolved Hide resolved
@mariari mariari marked this pull request as ready for review Feb 7, 2020
@mariari

This comment has been minimized.

Copy link
Contributor Author

mariari commented Feb 7, 2020

Currently tracing the `termToInstr=

We get something akin to this

let f a = (+) a

f 2 3
  1. [Name a push 2, push 3] -- from <- insts
  2. split into [2] [3]
  3. recurse (+) a
  4. [ Name (a, _1) push 2, Name _2 Push 3] -- from <- insts
    ...

We end up with [Push 2, Push 3, +]. Which is almost correct, namely we need to swap the first n arguments + needs.

Also we need to consider what happens in a partial application case on a primitive function, I think due to inconsistent naming that it would not end too well.

@cwgoes

This comment has been minimized.

Copy link
Collaborator

cwgoes commented Feb 9, 2020

Also we need to consider what happens in a partial application case on a primitive function, I think due to inconsistent naming that it would not end too well.

We could try giving the primitive functions named arguments, then reordering them on the stack in the order of the named arguments (e.g. alphabetical with _0, _1`, etc.) prior to inlining the primitive operation, I think that would work.

@cwgoes

This comment has been minimized.

Copy link
Collaborator

cwgoes commented Feb 9, 2020

Also, names _0, _1, etc. will need to be uniquely namespaced for each primitive function instance to deal correctly w/partial application.

mariari added 3 commits Feb 10, 2020
@mariari mariari mentioned this pull request Feb 11, 2020
1 of 7 tasks complete
@mariari mariari merged commit dcb4e81 into develop Feb 11, 2020
1 check was pending
1 check was pending
ci/circleci: setup_dependencies CircleCI is running your tests
Details
@mariari mariari deleted the mariari/michelson-changes branch Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.