DelphinusLab / zkWasm

Apache License 2.0
479 stars 95 forks source link

Simple programs crash with "not yet implemented" #70

Closed naps62 closed 1 year ago

naps62 commented 1 year ago

A lot of simple examples seem to fail to run due to todo!() tags. From a quick look, this seems to be on wasmi itself, not your repo. Essentially lots of instructions still yet to be implemented on that side As I'm not familiar with wasmi, not sure if there is a timeline to expect full compatibility?

Example:

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub fn parse() {
    let data = vec![0x83, b'c', b'a', b't'];
    let animal: String = rlp::decode(&data).expect("could not decode");
    assert_eq!(animal, "cat".to_owned());
}

fails with:

thread 'main' panicked at 'not yet implemented', wasmi/src/isa.rs:589:36

When checking wasmi/src/isa.rs it can be seen that a lot of instructions are still todo!()

laoliang2021 commented 1 year ago

Would you please be a bit more specific about what non-float instructions you are after? Some may have been tracked.

naps62 commented 1 year ago

This is the snippet where it crashed. I believe the specific instruction was I32Clz

            Instruction::F32Eq => todo!(),
            Instruction::F32Ne => todo!(),
            Instruction::F32Lt => todo!(),
            Instruction::F32Gt => todo!(),
            Instruction::F32Le => todo!(),
            Instruction::F32Ge => todo!(),
            Instruction::F64Eq => todo!(),
            Instruction::F64Ne => todo!(),
            Instruction::F64Lt => todo!(),
            Instruction::F64Gt => todo!(),
            Instruction::F64Le => todo!(),
            Instruction::F64Ge => todo!(),
            Instruction::I32Clz => todo!(),
            Instruction::I32Ctz => todo!(),
            Instruction::I32Popcnt => todo!(),
laoliang2021 commented 1 year ago

Ok, no problem. I think we do not have a concrete timeline for float point arith at the moment, but we will check Clz and Ctz.

junyu0312 commented 1 year ago

We have implemented ctz/clz instructions last week. https://github.com/DelphinusLab/zkWasm/pull/71

naps62 commented 1 year ago

@junyu0312 thanks, but this doesn't seem to fully do it.

Trying the same program (on the original issue) I now run into other unimplemented behaviour:

thread 'main' panicked at 'not yet implemented', wasmi/src/isa.rs:504:40

which seems to be on the Instruction::GrowMemory instruction