Virtual machine using RPython and PyPy

Question

I'm writing a virtual machine in Python using RPython and the PyPy toolchain. The RPython will still work in the ordinary Python 2 interpreter; it's just a bit slow unless it's compiled to C code with PyPy.

Does anyone have any positive or negative feedback about my VM?

The virtual machine has 4 instructions:

1. EOP - End of Program
2. EOI - End of Instruction
3. PUSH - Push item onto stack
4. PRINT - Print the item at the top of the stack

bcode = []
stack = []
regs = []
sp = 0
bcd = []

'''
Instructions
'''
OP_EOP = 0
OP_EOI = 1
OP_PUSH = 2
OP_PRINT = 3

def load_program(f2o):
    f = open(f2o, "r")
    f2 = f.read()
    f2 = f2.replace("\n"," ")
    bcode = f2.split(" ")
    i = 0
    for item in bcode:
        item = int(item, 16)
        bcd.append(item)
        i += 1
    return bcd

# VM action functions
def do_EOP():
    print "End of Program"

def do_PUSH(ba, b, ip):
    i = 2
    loop = 1
    cb = len(b)

    stack.insert(0, ba)

def do_PRINT(stack):
    stk = stack[0].split(" ")
    for item in stk:
        print unichr(int(item))

def execute_program(b):
    ip = 0
    sp = 0
    loop = 1
    cb = len(b)
    while (loop):
        if ip < cb:
            bc = b[ip]
            if bc == OP_PUSH:
                if bc != OP_EOI:
                    ba = str(b[ip + 1])
                    do_PUSH(ba, b, ip)
            elif bc == OP_PRINT:
                do_PRINT(stack)
            ip += 1
        else:
            loop = 0


def run_program(f):
    b = load_program(f)
    execute_program(b)

def main(argv):
    run_program(argv[1])
    return 0

def target(*args):
    return main, None

if __name__ == '__main__':
    import sys
    main(sys.argv)

Here's a file that includes the bytecode that prints "Hello World!" to the screen:

0002 0048 0001 0003
0002 0065 0001 0003
0002 006C 0001 0003
0002 006C 0001 0003
0002 006F 0001 0003
0002 0020 0001 0003
0002 0057 0001 0003
0002 006F 0001 0003
0002 0072 0001 0003
0002 006C 0001 0003
0002 0064 0001 0003
0002 0021 0001 0003

You can test the VM like this; it's written in Python 2.7:

python vm.py

Answer 1

bcode = []
stack = []
regs = []
sp = 0
bcd = []

Global variables like this are frowned upon. To be pythonic you should really put them a in a class or something.

'''
Instructions
'''
OP_EOP = 0
OP_EOI = 1
OP_PUSH = 2
OP_PRINT = 3

def load_program(f2o):

f2o? What in the world is that?

    f = open(f2o, "r")
    f2 = f.read()

My recollection of RPython is that it doesn't support this pattern. Are you sure it works in RPython?

    f2 = f2.replace("\n"," ")
    bcode = f2.split(" ")
    i = 0
    for item in bcode:
        item = int(item, 16)
        bcd.append(item)
        i += 1

What are you ding with i? You count it, but don't do anything with the value that you count.

    return bcd

# VM action functions
def do_EOP():
    print "End of Program"

def do_PUSH(ba, b, ip):

ba? b? pick variables names that let me know what they represent.

    i = 2
    loop = 1
    cb = len(b)

None of these last three lines do anything. They set variables local to the function when then gets thrown away.

    stack.insert(0, ba)

For a stack, we usually use the end of the list as the top of stack. That way you can simply append() and pop() the list. Its rather inefficient to insert at the begining.

def do_PRINT(stack):
    stk = stack[0].split(" ")
    for item in stk:
        print unichr(int(item))

def execute_program(b):
    ip = 0
    sp = 0
    loop = 1

Use True and False for true/false.

    cb = len(b)
    while (loop):

You don't need the ( or )

        if ip < cb:
            bc = b[ip]
            if bc == OP_PUSH:
                if bc != OP_EOI:

Are you expecting bc to change between those two lines?

                    ba = str(b[ip + 1])

You read the next piece of bytecode to push it, but you don't skip increment ip to account for it.

                    do_PUSH(ba, b, ip)

            elif bc == OP_PRINT:
                do_PRINT(stack)
            ip += 1
        else:
            loop = 0


def run_program(f):
    b = load_program(f)
    execute_program(b)

def main(argv):
    run_program(argv[1])
    return 0

def target(*args):
    return main, None

if __name__ == '__main__':
    import sys
    main(sys.argv)

The biggest issue with your code is that you have a lot of code that can't possibly do anything useful. Reading me leaves me to wonder if you just haven't cleaned up your code or are confused about the code actually works.

Answer 2

You need to close files you've open. From the Python doc :

When you’re done with a file, call f.close() to close it and free up any system resources taken up by the open file.

Even better, you should use the with keyword :

It is good practice to use the with keyword when dealing with file objects. This has the advantage that the file is properly closed after its suite finishes, even if an exception is raised on the way. It is also much shorter than writing equivalent try-finally blocks.

---

Don't use the same variable for two different things if you can avoid it as it can make it hard to understand the kind of data the variable is supposed to refer to. For instance, it's easier to get what f2 is like if you rewrite

f2 = f.read()
f2 = f2.replace("\n"," ")

with a single assignment to it

f2 = f.read().replace("\n"," ")

But ultimately, you don't need f2 at all.

---

The i variable does not seem to be useful.

---

I had troubles testing your code on your example because an empty string was messing with the conversion to int. I took the liberty to had a simple if item in the loop to deal with this but you can probably find other solutions depending on the input files you want to accept.

---

The whole load_program function can be rewritten in a more concise way with list comprehension :

def load_program(f2o):
    with open(f2o, "r") as f:
        bcode = f.read().replace("\n"," ").split(" ")
        return [int(item,16) for item in bcode if item]

---

Many variables are defined but not used : just get rid of them!

The same comment applies to unused parameters.

---

You could use the Boolean type for the loop variable. You could also get rid of it altogether by using break (this is a bit personal, some like it with a loop variable, I'd rather see avoid it).

---

Actually, going further, one can easily spot that this while loop is nothing but a for loop in disguise. What you want is just ip to go from 0 to len(cb)-1. The pythonic way to write this is :

for ip in range(len(b)):

Also, if what you want is to iterate over a container and also get the corresponding index, the pythonic way is :

---

ba = str(b[i + 1]) probably deserves some extra logic you don`t go too far.

---

In :

        if bc == OP_PUSH:
            if bc != OP_EOI:

you are comparing bc to 2 and if they are equal you compare bc to 1. As far as I can understand, the second check is not useful at all.

---

At this stage, my current version of the code is :

#!/usr/bin/python

stack = []

'''
Instructions
'''
OP_PUSH = 2
OP_PRINT = 3

def load_program(f2o):
    with open(f2o, "r") as f:
        bcode = f.read().replace("\n"," ").split(" ")
        return [int(item,16) for item in bcode if item]

# VM action functions
def do_EOP():
    print "End of Program"

def do_PUSH(ba):
    stack.insert(0, ba)

def do_PRINT(stack):
    stk = stack[0].split(" ")
    for item in stk:
        print unichr(int(item))


def execute_program(b):
    for i,bc in enumerate(b):
        if bc == OP_PUSH:
            ba = str(b[i + 1])
            do_PUSH(ba)
        elif bc == OP_PRINT:
            do_PRINT(stack)

def run_program(f):
    execute_program(load_program(f))

def main(argv):
    run_program(argv[1])
    return 0

def target(*args):
    return main, None

if __name__ == '__main__':
    import sys
    main(sys.argv)

but I'm realising that many things are probably wrong and nor nearly as tested as they should be giving the amount of things I've removed without any impact on the test you have provided.