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.

Sign up for GitHub

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

Jump to bottom

Off-by-one error when indexing individual pins of a bus #3078

Closed
gergoerdi opened this issue Nov 11, 2021 · 10 comments · Fixed by #3079
Closed

Off-by-one error when indexing individual pins of a bus #3078

gergoerdi opened this issue Nov 11, 2021 · 10 comments · Fixed by #3079
Assignees
@mwkmwkmwk

Comments

@gergoerdi
Copy link

Steps to reproduce the issue

Verilog source:

module Top(
           output [3:0] LED,
           inout [10:1] JA
           );
   
   wire [3:0]           ROWS = JA[10:7];  
   wire [3:0]           COLS = 4'b1110;
   
   assign LED = ROWS;
   assign JA[4:1] = COLS;   
   
endmodule

I have a keypad connected to the JA port, so some of the bits in the JA bus are output (to select the scanned columns) and some are input (to read in the currently selected column from all rows).

Expected behavior

The state of the selected (0-th) column should show up on the LEDs.

Actual behavior

If we look at the Yosys synthesis result, we can see that COLS gets assigned to JA[3:0] instead of JA[4:1]. Similarly, the pins read from JA are off by one, so instead of 10:7, ROWS (and consequently, LED) gets assigned from 9:6:

$ echo 'read_verilog Top.v; synth; show;' | yosys

Screenshot at 2021-11-11 22-41-26
@gergoerdi gergoerdi changed the title Mixed inout pin usage leads to off-by-one pin indexing error Off-by-one error when indexing individual pins of a bus Nov 11, 2021
@mwkmwkmwk
Copy link
Member

This is not an actual error, it's just idiosyncracy of show command (it always uses internal yosys bit numbering, which is from 0 LSB).

When written to JSON or other output format, the relevant wire should have the right annotations to be handled correctly

@gergoerdi
Copy link
Author

This is not an actual error, it's just idiosyncracy of show command (it always uses internal yosys bit numbering, which is from 0 LSB).

When written to JSON or other output format, the relevant wire should have the right annotations to be handled correctly

I'm not convinced. When I synthesize the whole circuit end-to-end, targeting a Xilinx 7 dev board, the only way I can get it to work is if I shift the definition of COLS by one, i.e. if I set COLS = 4'b1101, and then I can observe the 0th column (!) being scanned. Similarly, the input as read from ROWS is then shown off-by-one on the LEDs.

@mwkmwkmwk
Copy link
Member

Then please give an instruction for how to reproduce the problem end-to-end

gergoerdi added a commit to gergoerdi/yosys-symbiflow-plugins-issue-144 that referenced this issue Nov 11, 2021
@gergoerdi
Full reproducer for YosysHQ/yosys#3078
f7c430d
gergoerdi added a commit to gergoerdi/yosys-symbiflow-plugins-issue-144 that referenced this issue Nov 11, 2021
@gergoerdi
Full reproducer for YosysHQ/yosys#3078
1eb7b9a
@gergoerdi
Copy link
Author

Sure, here it is: https://github.com/gergoerdi/yosys-issue-3078/tree/1eb7b9a88b678087e7ace0e3ade7c45e13e39c34

Build with make, then upload to a Nexys A7-50T development board with make upload. Connect a Digilent 4x4 keypad to the JA user port, and then observe the LED behaviour.

@gergoerdi gergoerdi mentioned this issue Nov 11, 2021
Closed
@gergoerdi
Copy link
Author

gergoerdi commented Nov 12, 2021 •
edited

I have a much simplified version at https://github.com/gergoerdi/yosys-issue-3078/tree/81f4cd6878574614f08269e44ec8df72a857c640 that doesn't require any external peripherals:

module Top(
           output [5:2] LED
           );

   assign LED[5:2] = 4'b1101;   
endmodule

With Vivado this lights up LEDs 2, 4 and 5; with Symbiflow using Yosys, it lights up only LED 2.

$ echo 'read_verilog Top.v; synth_xilinx; show;' | yosys

Screenshot at 2021-11-12 17-30-46

@gatecat
Copy link
Member

gatecat commented Nov 12, 2021

I've got no idea what all the Symbiflow bits are doing, but running yosys -p "synth_xilinx -top Top -blif /dev/stdout" Top.v produces:

.model Top
.inputs
.outputs LED[2] LED[3] LED[4] LED[5]
.names $false
.names $true
1
.names $undef
.subckt OBUF I=$true O=LED[2]
.subckt OBUF I=$false O=LED[3]
.subckt OBUF I=$true O=LED[4]
.subckt OBUF I=$true O=LED[5]
.end

which looks like correct BLIF output to me

@gergoerdi
Copy link
Author

Thanks, I've reported this to the Symbiflow project as well.

@gatecat
Copy link
Member

gatecat commented Nov 12, 2021

I'd be curious what the eblif file from symbiflow_synth looks like if you could post it here (I don't have all their wrappers set up)

@gergoerdi
Copy link
Author

I'd be curious what the eblif file from symbiflow_synth looks like if you could post it here

# Generated by Yosys 0.9+4270 (git sha1 539d4ee9, x86_64-conda_cos6-linux-gnu-gcc 1.24.0.133_b0863d8_dirty -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -fdebug-prefix-map=/home/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/yosys_1628927583200/work=/usr/local/src/conda/yosys-0.9_5567_g539d4ee9 -fdebug-prefix-map=/home/cactus/.conda/envs/xc7=/usr/local/src/conda-prefix -fPIC -Os -fno-merge-constants)

.model Top
.inputs
.outputs LED[2] LED[3] LED[4] LED[5]
.subckt GND GND=$false
.subckt VCC VCC=$true
.subckt VCC VCC=$undef
.subckt T_INV TI=$true TO=$iopadmap$Top.LED.t
.cname $iopadmap$Top.LED.genblk1.t_inv
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1950.11-1953.6"
.subckt OBUFT_VPR I=$true O=LED[2] T=$iopadmap$Top.LED.t
.cname $iopadmap$Top.LED.obuft
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:2064.5-2068.4"
.param DRIVE 00000000000000000000000000001100
.param IOSTANDARD "LVCMOS33"
.param IO_LOC_PAIRS "NONE"
.param LVCMOS12_DRIVE_I12 0
.param LVCMOS12_DRIVE_I4 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SLEW_FAST 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SSTL135_SSTL15_SLEW_SLOW 1
.param LVCMOS12_LVCMOS15_LVCMOS18_SSTL135_SSTL15_STEPDOWN 0
.param LVCMOS12_LVCMOS25_DRIVE_I8 0
.param LVCMOS15_DRIVE_I12 0
.param LVCMOS15_DRIVE_I8 0
.param LVCMOS15_LVCMOS18_LVCMOS25_DRIVE_I4 0
.param LVCMOS15_SSTL15_DRIVE_I16_I_FIXED 0
.param LVCMOS18_DRIVE_I12_I8 0
.param LVCMOS18_DRIVE_I16 0
.param LVCMOS18_DRIVE_I24 0
.param LVCMOS25_DRIVE_I12 0
.param LVCMOS25_DRIVE_I16 0
.param LVCMOS33_DRIVE_I16 0
.param LVCMOS33_LVTTL_DRIVE_I12_I16 1
.param LVCMOS33_LVTTL_DRIVE_I12_I8 0
.param LVCMOS33_LVTTL_DRIVE_I4 0
.param LVTTL_DRIVE_I24 0
.param PULLTYPE "NONE"
.param PULLTYPE_KEEPER 0
.param PULLTYPE_NONE 1
.param PULLTYPE_PULLDOWN 0
.param PULLTYPE_PULLUP 0
.param SLEW "SLOW"
.param SSTL135_DRIVE_I_FIXED 0
.param SSTL135_SSTL15_SLEW_FAST 0
.subckt T_INV TI=$true TO=$iopadmap$Top.LED_1.t
.cname $iopadmap$Top.LED_1.genblk1.t_inv
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1950.11-1953.6"
.subckt OBUFT_VPR I=$false O=LED[3] T=$iopadmap$Top.LED_1.t
.cname $iopadmap$Top.LED_1.obuft
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:2064.5-2068.4"
.param DRIVE 00000000000000000000000000001100
.param IOSTANDARD "LVCMOS33"
.param IO_LOC_PAIRS "NONE"
.param LVCMOS12_DRIVE_I12 0
.param LVCMOS12_DRIVE_I4 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SLEW_FAST 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SSTL135_SSTL15_SLEW_SLOW 1
.param LVCMOS12_LVCMOS15_LVCMOS18_SSTL135_SSTL15_STEPDOWN 0
.param LVCMOS12_LVCMOS25_DRIVE_I8 0
.param LVCMOS15_DRIVE_I12 0
.param LVCMOS15_DRIVE_I8 0
.param LVCMOS15_LVCMOS18_LVCMOS25_DRIVE_I4 0
.param LVCMOS15_SSTL15_DRIVE_I16_I_FIXED 0
.param LVCMOS18_DRIVE_I12_I8 0
.param LVCMOS18_DRIVE_I16 0
.param LVCMOS18_DRIVE_I24 0
.param LVCMOS25_DRIVE_I12 0
.param LVCMOS25_DRIVE_I16 0
.param LVCMOS33_DRIVE_I16 0
.param LVCMOS33_LVTTL_DRIVE_I12_I16 1
.param LVCMOS33_LVTTL_DRIVE_I12_I8 0
.param LVCMOS33_LVTTL_DRIVE_I4 0
.param LVTTL_DRIVE_I24 0
.param PULLTYPE "NONE"
.param PULLTYPE_KEEPER 0
.param PULLTYPE_NONE 1
.param PULLTYPE_PULLDOWN 0
.param PULLTYPE_PULLUP 0
.param SLEW "SLOW"
.param SSTL135_DRIVE_I_FIXED 0
.param SSTL135_SSTL15_SLEW_FAST 0
.subckt T_INV TI=$true TO=$iopadmap$Top.LED_2.t
.cname $iopadmap$Top.LED_2.genblk1.t_inv
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1950.11-1953.6"
.subckt OBUFT_VPR I=$true O=LED[4] T=$iopadmap$Top.LED_2.t
.cname $iopadmap$Top.LED_2.obuft
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:2064.5-2068.4"
.param DRIVE 00000000000000000000000000001100
.param IOSTANDARD "LVCMOS33"
.param IO_LOC_PAIRS "LED[2]:J13"
.param LVCMOS12_DRIVE_I12 0
.param LVCMOS12_DRIVE_I4 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SLEW_FAST 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SSTL135_SSTL15_SLEW_SLOW 1
.param LVCMOS12_LVCMOS15_LVCMOS18_SSTL135_SSTL15_STEPDOWN 0
.param LVCMOS12_LVCMOS25_DRIVE_I8 0
.param LVCMOS15_DRIVE_I12 0
.param LVCMOS15_DRIVE_I8 0
.param LVCMOS15_LVCMOS18_LVCMOS25_DRIVE_I4 0
.param LVCMOS15_SSTL15_DRIVE_I16_I_FIXED 0
.param LVCMOS18_DRIVE_I12_I8 0
.param LVCMOS18_DRIVE_I16 0
.param LVCMOS18_DRIVE_I24 0
.param LVCMOS25_DRIVE_I12 0
.param LVCMOS25_DRIVE_I16 0
.param LVCMOS33_DRIVE_I16 0
.param LVCMOS33_LVTTL_DRIVE_I12_I16 1
.param LVCMOS33_LVTTL_DRIVE_I12_I8 0
.param LVCMOS33_LVTTL_DRIVE_I4 0
.param LVTTL_DRIVE_I24 0
.param PULLTYPE "NONE"
.param PULLTYPE_KEEPER 0
.param PULLTYPE_NONE 1
.param PULLTYPE_PULLDOWN 0
.param PULLTYPE_PULLUP 0
.param SLEW "SLOW"
.param SSTL135_DRIVE_I_FIXED 0
.param SSTL135_SSTL15_SLEW_FAST 0
.subckt T_INV TI=$true TO=$iopadmap$Top.LED_3.t
.cname $iopadmap$Top.LED_3.genblk1.t_inv
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1950.11-1953.6"
.subckt OBUFT_VPR I=$true O=LED[5] T=$iopadmap$Top.LED_3.t
.cname $iopadmap$Top.LED_3.obuft
.attr module_not_derived 00000000000000000000000000000001
.attr src "/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:1921.5-1925.4|/home/cactus/prog/fpga/symbiflow/symbiflow-examples/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_map.v:2064.5-2068.4"
.param DRIVE 00000000000000000000000000001100
.param IOSTANDARD "LVCMOS33"
.param IO_LOC_PAIRS "LED[3]:N14"
.param LVCMOS12_DRIVE_I12 0
.param LVCMOS12_DRIVE_I4 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SLEW_FAST 0
.param LVCMOS12_LVCMOS15_LVCMOS18_LVCMOS25_LVCMOS33_LVTTL_SSTL135_SSTL15_SLEW_SLOW 1
.param LVCMOS12_LVCMOS15_LVCMOS18_SSTL135_SSTL15_STEPDOWN 0
.param LVCMOS12_LVCMOS25_DRIVE_I8 0
.param LVCMOS15_DRIVE_I12 0
.param LVCMOS15_DRIVE_I8 0
.param LVCMOS15_LVCMOS18_LVCMOS25_DRIVE_I4 0
.param LVCMOS15_SSTL15_DRIVE_I16_I_FIXED 0
.param LVCMOS18_DRIVE_I12_I8 0
.param LVCMOS18_DRIVE_I16 0
.param LVCMOS18_DRIVE_I24 0
.param LVCMOS25_DRIVE_I12 0
.param LVCMOS25_DRIVE_I16 0
.param LVCMOS33_DRIVE_I16 0
.param LVCMOS33_LVTTL_DRIVE_I12_I16 1
.param LVCMOS33_LVTTL_DRIVE_I12_I8 0
.param LVCMOS33_LVTTL_DRIVE_I4 0
.param LVTTL_DRIVE_I24 0
.param PULLTYPE "NONE"
.param PULLTYPE_KEEPER 0
.param PULLTYPE_NONE 1
.param PULLTYPE_PULLDOWN 0
.param PULLTYPE_PULLUP 0
.param SLEW "SLOW"
.param SSTL135_DRIVE_I_FIXED 0
.param SSTL135_SSTL15_SLEW_FAST 0
.end

@mwkmwkmwk
Copy link
Member

I'll take care of the show issue; otherwise, this is a symbiflow bug

@mwkmwkmwk mwkmwkmwk self-assigned this Nov 12, 2021
mwkmwkmwk added a commit that referenced this issue Nov 12, 2021
@mwkmwkmwk
show: Fix wire bit indexing.
c9d394e 
Fixes #3078.
@mwkmwkmwk mwkmwkmwk mentioned this issue Nov 12, 2021
Merged
mwkmwkmwk added a commit that referenced this issue Nov 12, 2021
@mwkmwkmwk
show: Fix wire bit indexing.
107aad2 
Fixes #3078.
@gergoerdi gergoerdi mentioned this issue Nov 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mwkmwkmwk mwkmwkmwk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

show: Fix wire bit indexing. YosysHQ/yosys
3 participants
@gergoerdi @mwkmwkmwk @gatecat