clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.44k stars 153 forks source link

VIO output not connected correctly #2506

Closed leonschoorl closed 1 year ago

leonschoorl commented 1 year ago
module VIOtest where

import Clash.Prelude
import Clash.Cores.Xilinx.VIO
import Clash.Annotations.TH

type Dom = XilinxSystem

viotest ::
  "clk" ::: Clock Dom ->
  "inp" ::: Signal Dom Bit ->
  "led" ::: ("0" ::: Signal Dom Bit, "1" ::: Signal Dom Bit)
viotest clk inp = (out0, out1)
 where
  inNames = "my_probe_in" :> Nil
  outNames = "my_probe_out_0" :> "my_probe_out_1" :> Nil
  out = vioProbe @Dom inNames outNames (low,low) clk inp
  (out0,out1) = unbundle out

makeTopEntity 'viotest

generates:

/* AUTOMATICALLY GENERATED VERILOG-2001 SOURCE CODE.
** GENERATED BY CLASH 1.7.0. DO NOT MODIFY.
*/
`default_nettype none
`timescale 100fs/100fs
module viotest
    ( // Inputs
      input wire  clk // clock
    , input wire  inp

      // Outputs
    , output wire  led_0
    , output wire  led_1
    );
  // VIOtest.hs:22:1-7
  wire [1:0] ds;
  wire [1:0] led;

  assign led = {ds[1:1],   ds[0:0]};

  wire  probe_in0;
  wire  probe_out0;
  wire  probe_out1;
  (* KEEP = "true" *) wire [0:0] my_probe_out_0;
  (* KEEP = "true" *) wire [0:0] my_probe_out_1;
  (* KEEP = "true" *) wire [0:0] my_probe_in;
  assign probe_in0 = inp;

  assign my_probe_out_0 = probe_out0;

  assign my_probe_out_1 = probe_out1;

  assign my_probe_in = probe_in0;

  viotest_vioProbe_E5D466CB96E912F9 vio_inst
    ( .clk (clk)
    , .probe_in0 (my_probe_in)
    , .probe_out0 (my_probe_out_0)
    , .probe_out1 (my_probe_out_1) );

  assign ds = {probe_out0,   probe_out1};

  assign led_0 = led[1:1];

  assign led_1 = led[0:0];

endmodule

There are multiple related issues with routing of the vio outputs:

  1. the wires probe_out0 and probe_out1 aren't driven by anything
  2. the output led_0 and led_1 aren't driven either, (indirectly via led and ds they're assigned probe_out0 and probe_out1)
  3. my_probe_out_0 and my_probe_out_1 are driven twice, once directly with assign and once in the port mapping of the vio
leonschoorl commented 1 year ago

The weird thing is this code does work in practice on hardware. Which is probably why this wasn't noticed earlier.

If I read this code, I'd think the outputs led_0 and led_1 are undefined because they're effectively connected to nothing. And the simulator does seem to agree with that.

But Vivado is happy to synthesize it. (It does generate warnings about probe_out0 and probe_out1 not having any drivers) And when you add appropriate pin mappings, you can actually make 2 leds light up from the VIO GUI. It seems the synthesizer is happy to read the assign my_probe_out_0 = probe_out0; backwards, and effectively assign my_probe_out_0 to the wire probe_out0. I suspect Vivado might use in undirected graph as a netlist format somewhere internally.

martijnbastiaan commented 1 year ago

Amazing, nice find! Conclusion seems sensible.