albertodemichelis / squirrel

Official repository for the programming language Squirrel
http://www.squirrel-lang.org
MIT License
894 stars 148 forks source link

Compiler optimisation removes variable assignment #274

Closed samisalreadytaken closed 11 months ago

samisalreadytaken commented 11 months ago

Local variable assignment to adjacent local variables in comma separated declarations ignore the first variable's assignment and cause it to have no value.

// a = null
// b = 1
local a = 1, b = a;
-----LOCALS
[2] b   1 0
[1] a   1 0
[0] this    0 1
-----dump
[000]     _OP_LOADINT 2 1 0 0

The source of this issue is at SQFuncState::AddInstruction() https://github.com/albertodemichelis/squirrel/blob/7da4a6f196cc4e45f753a46cf0b0a9c125df34e2/squirrel/sqfuncstate.cpp#L519-L530

reached from SQCompiler::LocalDeclStatement() https://github.com/albertodemichelis/squirrel/blob/7da4a6f196cc4e45f753a46cf0b0a9c125df34e2/squirrel/sqcompiler.cpp#L1067

Removing if(pi._arg0 == i._arg1) block compiles it identical to local a = 1; local b = a; with optimisations.

-----LOCALS
[2] b   2 1
[1] a   1 1
[0] this    0 2
-----dump
[000]     _OP_LOADINT 1 1 0 0
[001]        _OP_MOVE 2 1 0 0

This can be observed with _OP_GET and other instructions listed there. _OP_GET actually assigns the GET-index to the variable.

this[1337] <- "x";

// a = 1337
// b = "x"
local a = this[1337], b = a;
-----LOCALS
[2] b   2 1
[1] a   2 1
[0] this    0 2
-----dump
[000]     _OP_LOADINT 1 1337 0 0
[001]         _OP_GET 2 0 1 0

With more variables

// a = 1
// b = null
// c = 1
local a = 1, b = 1, c = b;
-----LOCALS
[3] c   2 1
[2] b   2 1
[1] a   1 1
[0] this    0 2
-----dump
[000]     _OP_LOADINT 1 1 0 0
[001]     _OP_LOADINT 3 1 0 0

Two move instructions in a row are compiled into _OP_DMOVE, which works fine. Note how c is assigned to a, as assigning it to b causes the issue.

// a = 1
// b = 1
// c = 1
// d = 1
local a = 1, b = 1, c = a, d = c;
-----LOCALS
[4] d   3 2
[3] c   3 2
[2] b   2 2
[1] a   1 2
[0] this    0 3
-----dump
[000]     _OP_LOADINT 1 1 0 0
[001]     _OP_LOADINT 2 1 0 0
[002]       _OP_DMOVE 3 1 4 3
albertodemichelis commented 11 months ago

I'll look into this, thank you.

albertodemichelis commented 11 months ago

I fixed it