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

Add Call block to program MAST #375

Merged
merged 1 commit into from
Aug 26, 2022
Merged

Add Call block to program MAST #375

merged 1 commit into from
Aug 26, 2022

Conversation

bobbinth
Copy link
Contributor

@bobbinth bobbinth commented Aug 25, 2022

This PR adds a Call block to program MAST. This includes:

  • Added a code block table to Program struct. This creates a sort of a "virtual table" structure within a program such that internal nodes in the program MAST can refer to code blocks in the table. This is needed for handling call and syscall blocks, but could come in handing in other places too.
  • Updated assembly parsing to handle call statements. These look similar to exec statements, but semantically are different. While exec statements inline procedures into the main program MAST, call statements add an entry to the program's code block table and and adde a Call block node to the main program MAST.
  • Updated processor to handle call blocks. This includes updating the decoder to enable enable basic processing of call blocks.
  • Added a tiny test for to check that call blocks are executed correctly.

Surprisingly, these changes also speed up program compilation and execution time by 4% - 6%.

This PR does not yet include:

  • Memory context isolation.
  • Stack depth manipulation.
  • Documentation updates.

These will be addressed in future PRs.

@bobbinth bobbinth requested a review from grjte August 25, 2022 00:48
@bobbinth bobbinth mentioned this pull request Aug 25, 2022
26 tasks
Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may not have fully understood what's supposed to be happening, so I left a comment inline for clarification.

Also, while trying to get a better understanding of this I added a prove_and_verify test to the end of the simple local_fn_call test you created, and it failed.

Is this expected to fail at this point?

#[test]
fn local_fn_call() {
    let source = "
        proc.foo
            add
        end

        begin
            push.1
            push.2
            call.foo
        end";

    let test = build_test!(source);
    test.expect_stack(&[3]);

    test.prove_and_verify(vec![], 1, false);
}

Note: I tried testing the other control flow ops in the same way and flow_control::counter_controlled_loop also fails. The rest are fine. This is true before these changes as well, as expected. (In the integration tests flow control module)

core/src/program/mod.rs Outdated Show resolved Hide resolved
core/src/program/mod.rs Outdated Show resolved Hide resolved
processor/src/decoder/mod.rs Show resolved Hide resolved
processor/src/decoder/mod.rs Outdated Show resolved Hide resolved
@bobbinth
Copy link
Contributor Author

Is this expected to fail at this point?

So, this fails because the stack at the end is not 16. I've changed it to the below and it seems to work fine:

#[test]
fn local_fn_call() {
    let source = "
        proc.foo
            add
        end

        begin
            exec.foo
        end";

    let test = build_test!(source, &[1, 2]);
    test.expect_stack(&[3]);

    test.prove_and_verify(vec![1, 2], 1, false);
}

@grjte
Copy link
Contributor

grjte commented Aug 26, 2022

So, this fails because the stack at the end is not 16. I've changed it to the below and it seems to work fine:

Ah great, good point, should've realized that 😅 I'll be happy to see the end of that requirement!

Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thank you! I just left one nit inline for a typo in a doc comment

core/src/program/mod.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants