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

Slot opcodes #246

Closed
erikzhang opened this issue Dec 4, 2019 · 18 comments · Fixed by #264
Closed

Slot opcodes #246

erikzhang opened this issue Dec 4, 2019 · 18 comments · Fixed by #264
Labels

Comments

@erikzhang
Copy link
Member

In NEO2, we have alt stack and several opcodes such as TOALTSTACK and FROMALTSTACK to support parameters, local variables, and static fields. But since we are not going to be compatible with NEO2, we can remove the alt stack.

By adding some new opcodes, we can make access to parameters, local variables, and static fields easier:

INITSLOT //Initialize the argument slot and the local variable list for the current execution context.
INITSSLOT //Initialize the static field list for the current execution context.
LDARG //Loads the argument at a specified index onto the evaluation stack.
STARG //Stores the value on top of the evaluation stack in the argument slot at a specified index.
LDLOC //Loads the local variable at a specified index onto the evaluation stack.
STLOC //Stores the value on top of the evaluation stack in the local variable list at a specified index.
LDSFLD //Loads the static field at a specified index onto the evaluation stack.
STSFLD //Stores the value on top of the evaluation stack in the static field list at a specified index.

The argument slot and local variable list will be automatically cleaned up when RET is executed. The static field list will be automatically cleaned up when the last execution context belonging to the current contract is unloaded.

This way, we can completely remove alt stack and related opcodes. The implementation of the compiler will also be more convenient.

@shargon
Copy link
Member

shargon commented Dec 4, 2019

I understand that it will reduce opcodes and it could be faster, but could yo specify how will work the argument slot? it will be an array?

@erikzhang
Copy link
Member Author

After INITSLOT, 2 Arrays will be created in the CurrentContext.
After INITSSLOT, 1 Array will be created in the CurrentContext. If the context is created by CALL, the static field slot array will be shared.

Other LD* and ST* opcodes will access these Arrays.

@shargon
Copy link
Member

shargon commented Dec 4, 2019

Is good to me, I think that it will improve the execution flow.

@lightszero
Copy link
Member

so we have argumentlist,slotlist,staticslotlist on executecontext?
I found argument slot.
so call will automatic create a argumentlist?
so ldarg can work.
starg is only for out param like void abc(out int p)
so maybe we donot need starg

INITSLOT is not nessessary

STLOC or STSFLD can init it automatic.

@erikzhang
Copy link
Member Author

erikzhang commented Dec 4, 2019

Initially all the parameters are on the stack, you need INITSLOT to initialize the argument list. CALL won't create argument list automatically. Because it doesn't know the count of parameters.

STARG is nessessary too. See the following code:

int min(int x, int y)
{
    if (x > y) x = y;
    return x;
}

@lightszero
Copy link
Member

lightszero commented Dec 4, 2019

if CALL do not create param list

Push1
Push2
Call xxx(a,b)

?where fill 1 2 to paramlist?

insert code at beginning of xxx?
//2,1 in stack
INITSLOT
push 0
starg
push 1
starg

@erikzhang
Copy link
Member Author

erikzhang commented Dec 4, 2019

CALL won't create argument list automatically. The compiler should generate the INITSLOT at the begining of the method.

@erikzhang
Copy link
Member Author

erikzhang commented Dec 4, 2019

// stack: {2, 1}
PUSH 2 //parameters count
PUSH 0 //local variables count
INITSLOT //pop the 2 parameters and create the slots

@lightszero
Copy link
Member

lightszero commented Dec 4, 2019

ok,got it.
INITSLOT create parameterslist and slot list,how about slot list length.
ok,see it. two params

@erikzhang
Copy link
Member Author

At the beginning of the entry point, the compiler should generate INITSSLOT code to initialize the static field list.

@lightszero
Copy link
Member

Cool,more like c# now.

@vncoelho
Copy link
Member

vncoelho commented Dec 4, 2019

NEO RAM Memory

@igormcoelho
Copy link
Contributor

More like wasm as well. So we move from altstack model to linear memory... both work for me. I guess altstack is used on 99.9% of the time just for a single element or two, so it doesnt make sense indeed.
On classic stack based languages, double inner loops would occupy double space on altstack, and so on,but Neo resolves this with a single variable array. So, it is promising 👍

@igormcoelho
Copy link
Contributor

igormcoelho commented Dec 4, 2019

If we use so much double pushes, I think we could create an opcode DPUSH 02 00 that pushes two next bytes on stack. Its very common pattern, and will be more. Agree with static args and local structure.

@erikzhang
Copy link
Member Author

The two parameters could be the operand of INITSLOT. We don't need to push them onto the stack at all.

@igormcoelho
Copy link
Contributor

igormcoelho commented Dec 4, 2019

Better idea 😂 its also much safer... otherwise local buffers will get unpredictable sizes, terrible for optimizers and bug checkers. I'd like to see same strategy for args index loads and stores as well,so we easily guarantee var structure is correct.

@roman-khimov
Copy link
Contributor

Quite radical change, but maybe interesting. One question that I have is what's the rationale for these three slots other than it resembles CLR? Maybe we are fine with just one? If we're to solve ALT stack problem here (in that it's hardly useful for more than 1-2 elements) then adding random-access fixed-size scratchpad array maybe can solve it, but I think one array is enough for that.

@erikzhang erikzhang changed the title Improvements on accessing parameters, local variables, and static fields Slot opcodes Dec 6, 2019
@igormcoelho
Copy link
Contributor

I think three is just to simplify access, and avoid counts... otherwise we mix local, global and params. Space complexity is the same, so it may help us simplify.

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

Successfully merging a pull request may close this issue.

6 participants