Uncurried BiSignalIn params lead to faulty Verilog

[gergoerdi]

module Bi (topEntity) where

import Clash.Prelude
import Clash.Annotations.TH

topEntity
    :: "CLK_50MHZ" ::: Clock System
    -> "RESET"     ::: Reset System
    -> ( "SCL" ::: BiSignalIn 'PullUp System (BitSize Bit)
       , "SDA" ::: BiSignalIn 'PullUp System (BitSize Bit)
       )
    -> ( "SCL" ::: BiSignalOut 'PullUp System (BitSize Bit)
       , "SDA" ::: BiSignalOut 'PullUp System (BitSize Bit)
       )
topEntity clk reset = exposeClockResetEnable run clk reset enableGen

run
    :: (HiddenClockResetEnable dom)
    => ( BiSignalIn 'PullUp dom (BitSize Bit)
       , BiSignalIn 'PullUp dom (BitSize Bit)
       )
    -> ( BiSignalOut 'PullUp dom (BitSize Bit)
       , BiSignalOut 'PullUp dom (BitSize Bit)
       )
run (sclIn, sdaIn) = (sclOut, sdaOut)
  where
    sclOut = writeToBiSignal sclIn sclOut'
    sdaOut = writeToBiSignal sdaIn sdaOut'

    sclOut' = pure $ Just high
    sdaOut' = pure $ Just low

makeTopEntityWithName 'topEntity "bisignal"

/* AUTOMATICALLY GENERATED VERILOG-2001 SOURCE CODE.
** GENERATED BY CLASH 1.6.1. DO NOT MODIFY.
*/
`timescale 100fs/100fs
module bisignal
    ( // Inputs
      input  CLK_50MHZ // clock
    , input  RESET // reset
    , inout [0:0] SCL
    , inout [0:0] SDA

      // No outputs
    );
  // src/Bi.hs:19:1-9
  wire [0:0] sclIn;
  // src/Bi.hs:19:1-9
  wire [0:0] sdaIn;
  wire [1:0] ds;

  assign ds = {SCL,   SDA};

  assign sclIn = ds[1:1];

  assign sdaIn = ds[0:0];

  // writeToBiSignal# begin
  assign sdaIn = (1'b1 == 1'b1) ? 1'b0 : {1 {1'bz}};
  // writeToBiSignal# end

  // writeToBiSignal# begin
  assign sclIn = (1'b1 == 1'b1) ? 1'b1 : {1 {1'bz}};
  // writeToBiSignal# end


endmodule

[gergoerdi]

Note that scalIn is never written back to SCL.

[gergoerdi]

The curried version looks OK:

topEntity
    :: "CLK_50MHZ" ::: Clock System
    -> "RESET"     ::: Reset System
    -> "SCL" ::: BiSignalIn 'PullUp System (BitSize Bit)
    -> "SDA" ::: BiSignalIn 'PullUp System (BitSize Bit)
    -> ( "SCL" ::: BiSignalOut 'PullUp System (BitSize Bit)
       , "SDA" ::: BiSignalOut 'PullUp System (BitSize Bit)
       )
topEntity clk reset = exposeClockResetEnable run clk reset enableGen

run
    :: (HiddenClockResetEnable dom)
    => BiSignalIn 'PullUp dom (BitSize Bit)
    -> BiSignalIn 'PullUp dom (BitSize Bit)
    -> ( BiSignalOut 'PullUp dom (BitSize Bit)
       , BiSignalOut 'PullUp dom (BitSize Bit)
       )
run sclIn sdaIn = (sclOut, sdaOut)
  where
    sclOut = writeToBiSignal sclIn sclOut'
    sdaOut = writeToBiSignal sdaIn sdaOut'

    sclOut' = pure $ Just high
    sdaOut' = pure $ Just low

makeTopEntityWithName 'topEntity "bisignal"

module bisignal
    ( // Inputs
      input  CLK_50MHZ // clock
    , input  RESET // reset
    , inout [0:0] SCL
    , inout [0:0] SDA

      // No outputs
    );


  // writeToBiSignal# begin
  assign SDA = (1'b1 == 1'b1) ? 1'b0 : {1 {1'bz}};
  // writeToBiSignal# end

  // writeToBiSignal# begin
  assign SCL = (1'b1 == 1'b1) ? 1'b1 : {1 {1'bz}};
  // writeToBiSignal# end


endmodule

[gergoerdi]

I'm using Clash 1.6.1, and for boring reasons can't try out 1.6.3 right now; do let me know if there's been relevant changes in between.

[martijnbastiaan]

No changes related to that. Thanks for the report!

[leonschoorl]

Seems clash is supposed to report an error when arguments are of composite types containing BiSignalIns, like a tuple containing two BiSignalIns.

clash-compiler/clash-lib/src/Clash/Netlist/Util.hs

Lines 970 to 978 in acf574a

	-- * Throws an error if the port is a composite type with a BiSignalIn
	idToInPort :: Id -> NetlistMonad (Maybe (Identifier, HWType))
	idToInPort var = do
	(_, sp) <- Lens.use curCompNm
	portM <- idToPort var
	case portM of
	Just (_,hty) -> do
	when (containsBiSignalIn hty && not (isBiSignalIn hty))
	(throw (ClashException sp ($(curLoc) ++ "BiSignalIn currently cannot be part of a composite type when it's a function's argument") Nothing))

But somehow this error isn't being triggered.

[christiaanb]

That function is only called for non-topentities. If you add a NOINLINE pragma for run the error should trigger.

[christiaanb]

We could try to make BiSignalIn non-representable and see if the testsuite still passes:

clash-compiler/clash-lib/src/Clash/Netlist/Util.hs

Line 661 in acf574a

BiDirectional _ t -> isRepresentable t

that’s the easiest way to force Clash to inline the things we want inlined.

[jakobgross]

A similar issue arises when packing the BiSignalIn into some structure.
In this example, I wanted to produce an IO where individual bits could be controlled, which is not possible with the standard BiSignalIn 'Floating dom n.
No InOut ports are generated and no error is triggered.

topEntityComparisonIoBuffer ::
  Clock XilinxSystem ->
  Reset XilinxSystem ->
  Enable XilinxSystem ->
  Signal XilinxSystem (BitVector 8) -> -- Input from FSM
  Signal XilinxSystem (BitVector 8) -> -- Input Direction
  Vec 8 (BiSignalIn 'Floating XilinxSystem 1) -> -- Input from Outside
  (Signal XilinxSystem (BitVector 8), Vec 8 (BiSignalOut 'Floating XilinxSystem 1)) -- (Output to FSM, Output to Outside)
topEntityComparisonIoBuffer clk rst en = withClockResetEnable clk rst en ioBuffer

ioBuffer ::
  forall dom regSize.
  (HiddenClockResetEnable dom, KnownNat regSize) =>
  Signal dom (BitVector regSize) -> -- Input from FSM
  Signal dom (BitVector regSize) -> -- Input Direction
  Vec regSize (BiSignalIn 'Floating dom 1) -> -- Input from Outside
  (Signal dom (BitVector regSize), Vec regSize (BiSignalOut 'Floating dom 1)) -- (Output to FSM, Output to Outside)
ioBuffer inp inpD inpO = (outFsm, outO)
  where
    -- Change Signal BitVector to Vec (Signal Bit)
    vecEnableBits = unbundle $ bv2v <$> inpD

    -- Read Bit i only if inpD[i] is high
    readIfBit :: Signal dom Bit -> BiSignalIn 'Floating dom 1 -> Signal dom Bit
    readIfBit rd i = mux (bitToBool <$> rd) (readFromBiSignal i) undefined-- shoud generate "-" in vhdl when Bit should not be read
    readEnabledBits = zipWith readIfBit vecEnableBits inpO

    -- Bundle the read bits from Vec (Signal Bit) to Signal BitVector
    outFsm = v2bv <$> bundle readEnabledBits

    -- Write Bit i only if inpD[i] is low
    -- writeToBiSignal will only write on a Just value
    -- Enable, valuetoWrite, bisignal -> bisignalout
    writeIfBit :: Signal dom Bit -> Signal dom Bit -> BiSignalIn 'Floating dom 1 -> BiSignalOut 'Floating dom 1
    writeIfBit wr dat i = writeToBiSignal i (bitsToMaybe <$> wr <*> dat)

    bitsToMaybe :: Bit -> Bit -> Maybe Bit
    bitsToMaybe b1 b2 = if bitToBool b1 then Just b2 else Nothing

    vecInputBits = unbundle $ bv2v <$> inp
    outO = zipWith3 writeIfBit vecEnableBits vecInputBits inpO