TOPLLab / WARDuino-VSCode

🕵️ A VSCode debugger plugin for WARDuino.
Mozilla Public License 2.0
2 stars 2 forks source link

Fix `asc` compilation #122

Closed tolauwae closed 1 year ago

tolauwae commented 1 year ago

:sparkles: Changes

carllocos commented 1 year ago

As explained here https://github.com/TOPLLab/WARDuino-VSCode/issues/120#issuecomment-1633805074, I push the fix for issue #120 in this PR.

@tolauwae When time allows can you please confirm that this PR also fixes the problem for you? I'll then accept the PR.

tolauwae commented 1 year ago

~@carllocos it appears to be fixed~

tolauwae commented 1 year ago

@carllocos No it does not. I don't think the runtime argument is related to the issue.

I have isolated a small piece of code that causes the bug, but it doesn't always fail. Haven't tracked down when it does work:

export function invert(mode: PinVoltage): PinVoltage {
    if (mode === PinVoltage.LOW) {
        return PinVoltage.HIGH;
    } else {
        return PinVoltage.LOW;
    }
}

This is compiled to an implicit if, but it should be typed with (result i32). So the correct WebAssembly code should be:

 (func $invert (param $mode i32) (result i32)
  (if (result i32)
   (i32.eq
    (local.get $mode)
    (global.get $PinVoltage.LOW)
   )
   (return
    (global.get $PinVoltage.HIGH)
   )
   (return
    (global.get $PinVoltage.LOW)
   )
  )
 )

This piece of code only fails for non-optimized compilation on my machine. asc = 0.27.5

tolauwae commented 1 year ago

Also this was not a problem in the old version we were using (0.17.14). So I have reverted to the main branch for the demo. @carllocos so feel free to continue working on this branch.

carllocos commented 1 year ago

@tolauwae This is a peculiar problem. Not necessarily related to the options passed to the asc compiler. I will open an issue on asc repo. Hopefully, they can help me.

carllocos commented 1 year ago

@tolauwae can you please provide me with the source code of your example here above

I have isolated a small piece of code that causes the bug, but it doesn't always fail.

When it fails is it for the same type error reason as mentioned here https://github.com/TOPLLab/WARDuino-VSCode/issues/120#issue-1793499855?

tolauwae commented 1 year ago

You mean an example that uses it? I can't seem to find it. But as I remember the problem was with the invert function. So any code that uses it, should not compile.

The compile error was:

type mismatch in implicit if, expected [i32] but got []

The wasm code I posted above is the correct .wat code, but asc leaves out the typing of the if. Which causes the wat2wasm compilation error.

I do know a newer version of the AssemblyScript compiler has been released since, maybe it is fixed by now.

tolauwae commented 1 year ago

@carllocos just had a thought. If we use the source-map npm package to get the source mapping straight from AssemblyScript's own mapping, ie output.wasm.map, we don't need to get the .wast file anymore or compile it with wat2wasm.

This way we can potentially circumvent the problem. Since the binary generated by asc does appear to be fully up to spec.

And to be honest this was already a low priority TODO, since this is a faster, cleaner, and more stable approach.

I have a day off today, so I will have a stab at the refactor on Monday.

carllocos commented 1 year ago

@tolauwae that is an excellent idea! However, I tried to work with the same source mapper some time ago but without success :/. The mappings seemed to be only between AssemblyScript and Javascript. I do not know exactly how to get the mappings between Wasm and AssemblyScript. If you have time maybe you can look at it. Maybe there is something that I missed.

tolauwae commented 1 year ago

That would be a problem. I'll look in into it.

tolauwae commented 1 year ago

@carllocos I refactored to use the AS source map. That works on my end, and the compilation problem should be circumvented now.