nand2tetris / web-ide

A web-based IDE for https://nand2tetris.org
https://nand2tetris.github.io/web-ide
Other
38 stars 11 forks source link

[bug]: Order of test set operations affects result when using subbus on input pin #225

Closed dkilfoyle closed 4 months ago

dkilfoyle commented 4 months ago

Program

Hardware Simulator

Interface

Website (https://nand2tetris.github.io/web-ide)

Contact Details

No response

What happened?

Hi. The following snippet produces errors purely dependent on the order in which inputs are set in a test statement.

CHIP ALU {
    IN  
        x[16],
        zx, // zero the x input?
        nx; // negate the x input?
    OUT 
        out[16]; // 16-bit output

    PARTS:
    Not16(in=x, out=notx);

    // select x
    // zx nx  out
    // 0  0   x
    // 0  1   !x
    // 1  0   0
    // 1  1   !0 = 1111111111111111 
    Mux4Way16(a=x, b=notx, c[0..15]=false, d[0..15]=true, sel[0]=nx, sel[1]=zx, out=out);
}
output-list x%B1.16.1 zx%B1.1.1 zx%B1.1.1 out%B1.16.1;

set x %B000000000010001;  // x = 17

// Compute x
set zx 0, set nx 0, eval, output; // out = 17
set nx 0, set zx 0, eval, output; // out = 17 

// Compute !x
set zx 0, set nx 1, eval, output;  // out = -18
set nx 1, set zx 0, eval, output;  // out = -18

// Compute 0
set nx 0, set zx 1, eval, output;  // out = 0
set zx 1, set nx 0, eval, output;  // out = 17 (wrong! - behaving as if zx=0)

// Compute -1
set nx 1, set zx 1, eval, output;  // out = -1
set zx 1, set nx 1, eval, output;  // out = -18 (wrong! - behaving as if zx=0)

This anomaly does not occur in the Java version.

Additional Comments

No response

Do you want to try to fix this bug?

Code of Conduct

dkilfoyle commented 4 months ago

Some further investigation suggests the issue arises because sel[0]=nx is indistinguishable from sel=nx and the connection is therefore not wrapped in an InSubBus. A not very elegant possible solution is to add a subbed0 boolean to PinSide which is true when PinParts.start = 0 rather than undefined. Chip.wireInPin can then detect this with:

    // Wrap the partPin in an InBus when the part side is dimensioned
    if (to.start > 0 || to.width !== chipPin.width || to.subbed0) {
      partPin = new InSubBus(partPin, to.start, to.width);
    }

Sorry, I don't know enough about the build system to know how to test this or submit as a PR.