Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

New slot opcodes #264

Merged
merged 14 commits into from Dec 14, 2019
Merged

New slot opcodes #264

merged 14 commits into from Dec 14, 2019

Conversation

erikzhang
Copy link
Member

Closes #246

@erikzhang
Copy link
Member Author

@shargon Can you help with the UT? I don't know how to check the slots in the json.

@shargon
Copy link
Member

shargon commented Dec 12, 2019

I will do it

@erikzhang
Copy link
Member Author

Ready to merge.

Copy link
Contributor

@lock9 lock9 left a comment

Choose a reason for hiding this comment

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

I don't have enough knowledge to approve this PR, but I can definitely say that I love it. It simplifies a lot the VM, making it much easier for other developers to understand, maintain and maybe even update.
Again, I loved this PR, I wish I had enough knowledge to approve it.

src/neo-vm/OpCode.cs Show resolved Hide resolved
/// </summary>
FROMALTSTACK = 0x6C,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any operation that takes value from Slot and "removes" it? I mean, takes from last index of slot and reduces its size? It could be similar to the FROMALTSTACK... but I guess on practice we won't need it, compilers should be able to generate slot logic directly, instead of double stacks.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Slot size can't be changed.

ExecuteTest(ut);
try
{
ExecuteTest(ut);
Copy link
Contributor

Choose a reason for hiding this comment

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

New exceptions being launched now @shargon ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, because was hard to detect for me which file was wrong.

Copy link
Contributor

@igormcoelho igormcoelho left a comment

Choose a reason for hiding this comment

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

Amazing work! interesting shift from double-stack model to a slot model. Easier for compilers and better stack utilization.

shargon
shargon previously approved these changes Dec 14, 2019
@shargon
Copy link
Member

shargon commented Dec 14, 2019

INITSLOT 0x0000 is like a NOP, maybe we should fault on it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slot opcodes
4 participants