perf bug[venom]: unlifted variables in mem2var

[charles-cooper]

Version Information

• vyper Version (output of vyper --version): 6606ded

What's your issue about?

examples of unlifted variables:

• loop variable

@external
def loop(x: uint256) -> uint256:
    res: uint256 = 9
    for i: uint256 in range(res, bound=10):
        res = res + i
    return res

• unlifted assignment

@external
def foo(x: uint256) -> uint256:
    t: uint256 = 5 // x
    return 6 // x

How can it be fixed?

Fill this in if you know how to fix it.

[harkal]

Looks like the "transpiler" is not generating the appropriate allocas

[charles-cooper]

hmm. we should probably make sure in the transpiler that allocas get inserted in the entry block.

[harkal]

Looked into it. The alloca is fine. We need to do memory analysis to lift those mstores.

[charles-cooper]

i think the check for all(use.opcode == "mstore") for use in uses check in mem2var is blocking the second example:

vyper/vyper/venom/passes/mem2var.py

Lines 43 to 44 in 6843e79

	elif all([inst.opcode == "mstore" for inst in uses]):
	return

[charles-cooper]

this is blocking the first example, since the venom code contains a store of the alloca variable (%ret_ofst = %11):

vyper/vyper/venom/passes/mem2var.py

Line 45 in 6843e79

elif all([inst.opcode in ["mstore", "mload", "return"] for inst in uses]):

[harkal]

Right. But it is not safe to lift those atm, as it fails. If alloca's were super correct the "all mstores" should have not been a problem, but it probably not correct :/

[charles-cooper]

Right. But it is not safe to lift those atm, as it fails. If alloca's were super correct the "all mstores" should have not been a problem, but it probably not correct :/

my diagnosis is:

• the first two guards in mem2var protect the calling convention, they are overly conservative. we should be able to add analysis to check which mloads/mstores are part of the calling convention
• ret_ofst should probably not be magical, let me look into this more