Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hdl.ast.Value.word_select() works incorrectly on actual platform (ECP5 Versa) #310

Closed
nmigen-issue-migration opened this issue Jan 23, 2020 · 2 comments

Comments

@nmigen-issue-migration
Copy link

Issue by HarryHo90sHK
Thursday Jan 23, 2020 at 10:39 GMT
Originally opened as m-labs/nmigen#310


Issue description

According to its docstring, the function word_select(offset, width) of the class nmigen.hdl.ast.Value should return the slice [offset*width:(offset+1):width] of the Value subject. It returns such an expected result in simulation (using PySim), but does not when used on an actual platform, such as the ECP5 Versa evaluation board.

Steps to reproduce the issue

  1. Prepare the following nMigen script:

    import argparse
    import os
    
    from nmigen import *
    from nmigen.back import pysim
    from nmigen_boards.versa_ecp5 import VersaECP5Platform
    
    
    class Debug(Elaboratable):
        def __init__(self, simulate):
            self.simulate = simulate
    
        def elaborate(self, platform):
            m = Module()
    
            if self.simulate:
                leds = Signal(8)
            else:
                cd_sync = ClockDomain(reset_less=True)
                m.domains += cd_sync
                m.d.comb += cd_sync.clk.eq(platform.request("clk100").i)
                leds = [platform.request("led", i).o for i in range(8)]
    
            dummy = Signal(8)
            # Note: on the board I'm using, the LED outputs seem to be inverted
            #       e.g. leds[0].eq(1) --> LED#0 turns OFF
            #       e.g. leds[0].eq(0) --> LED#0 turns ON
    
            # Using word_select
            m.d.comb += [
                dummy.word_select(0, 2).eq(~0b00),
                dummy.word_select(1, 2).eq(~0b10),
                dummy.word_select(2, 2).eq(~0b11),
                dummy.word_select(3, 2).eq(~0b01)
            ]
    
            # Using slices
            m.d.comb += [
                dummy[0:2].eq(~0b00),
                dummy[2:4].eq(~0b10),
                dummy[4:6].eq(~0b11),
                dummy[6:8].eq(~0b01)
            ]
    
            m.d.sync += [
                leds[i].eq(dummy[i]) for i in range(0, 8)
            ]
    
            return m
    
    
    if __name__ == "__main__":
        parser = argparse.ArgumentParser()
        parser.add_argument("--simulate", action="store_true")
        build_dir = "./result"
        args = parser.parse_args()
    
        platform = VersaECP5Platform()
        top = Debug(args.simulate)
    
        if args.simulate:
            os.makedirs(build_dir, exist_ok=True)
            with pysim.Simulator(top,
                    vcd_file=open(os.path.join(build_dir, "debug.vcd"), "w"),
                    gtkw_file=open(os.path.join(build_dir, "debug.gtkw"), "w")) as sim:
                sim.add_clock(1e-6)
                sim.run_until(1000e-6, run_passive=True)
        else:
            platform.build(top, build_dir=build_dir)
  2. Comment out the block starting with the comment line "Using slices". Now we are testing with word_select.

  3. Under an environment with nMigen 0.1rc1 and other necessary tools (such as nextpnr, Trellis and OpenOCD) properly installed, run the script with Python. Add --simulate as the argument to see the VCD from simulation.

  4. Use OpenOCD to program the ECP5 board, given that a working Config file is ready (see the example versa.cfg from HeavyX's README). For example, use the following command after building the bitstream:

openocd -f versa.cfg -c "transport select jtag; init; svf result/top.svf; exit"
  1. The LEDs lights up like below (inverted bit pattern == 11101100):
    20200123_181111 8330324

  2. Now, repeat Step 2 but instead of commenting out the "Using slices" block, comment out the "Using word_select" this time. Now we are testing with slicing.

  3. Repeat Steps 3 and 4.

  4. The LEDs lights up like below (inverted bit pattern == 01111000):
    20200123_181312 8330311

Comparison between Actual and Expected Results

If we compare the simulation result (i.e. VCDs) from the two scenarios, they are the same and as expected (bit pattern == 10000111 == 01111000 inverted (due to LED polarity issue)):
image

However, the actual LED result when using word_select() is inconsistent with the simulation result, as revealed in the picture of Step 5.

Using LED is just an example to quickly show this issue. I have actually encountered the issue when using word_select() to grab slices over in nmigen-soc (link). So maybe it is a potential bug in nMigen?

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Thursday Jan 23, 2020 at 10:41 GMT


Interesting. It could be a bit width issue. Could you try to reproduce this by simulating in iverilog?

@whitequark whitequark added this to the 0.2 milestone Jan 31, 2020
@whitequark whitequark added duplicate and removed bug labels Feb 6, 2020
@whitequark whitequark removed this from the 0.2 milestone Feb 6, 2020
@whitequark
Copy link
Member

@HarryHo90sHK What you discovered is indeed a bug in nMigen. However, it is a bug in nMigen 0.1rc1 only; it is already fixed in 0.1 (and, of course, master). The commit that fixed it is ffd10e3.

Is there a reason you were using the version 0.1rc1 specifically? nmigen-soc does have a dependency on nmigen~=0.1.rc1, but note that the ~= version requirement is a SemVer requirement that accepts any compatible version, which includes 0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants