clash-compiler icon indicating copy to clipboard operation
clash-compiler copied to clipboard

Incorrectly using multiple port annotations leads to unexpected error

Open lmbollen opened this issue 1 year ago • 4 comments

Whenever you accidentally give the some type multiple port annotations, the Template Haskell code incorrectly matches the port types. This reproducer:

{-# OPTIONS_GHC -ddump-splices #-}
module Example.Project where

import Clash.Prelude
import Clash.Annotations.TH (makeTopEntity)

topEntity ::
  "A" ::: Signal System ("B" ::: ("C" ::: Bool, "D" ::: Bool))
topEntity = undefined

makeTopEntity 'topEntity

Leads to this error(generated Synthesize annotation included):

src/Example/Project.hs:11:1-24: Splicing declarations
    makeTopEntity 'topEntity
  ======>
    {-# ANN topEntity Synthesize
                        {t_name = "topEntity", t_inputs = [],
                         t_output = (PortProduct "A")
                                      [(PortProduct "B") [PortName "C", PortName "D"]]} #-}
Preprocessing executable 'clash' for simple-0.1..
Building executable 'clash' for simple-0.1..
Loaded package environment from /home/lucas/clash-starters/simple/.ghc.environment.x86_64-linux-9.0.2
GHC: Setting up GHC took: 0.180s
GHC: Compiling and loading modules took: 0.083s
Clash: Parsing and compiling primitives took 0.128s
GHC+Clash: Loading modules cumulatively took 0.451s
Clash: Compiling Example.Project.topEntity
Clash: Normalization took 0.008s

<no location info>: error:
    Clash error call:
    Saw a PortProduct in a Synthesize annotation:
    
      PortProduct "A_B" [PortName "C",PortName "D"]
    
    but the port type:
    
      Bool
    
    is not a product!
    CallStack (from HasCallStack):
      error, called at src/Clash/Netlist/Util.hs:1746:8 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist.Util
      expandTopEntityOrErr, called at src/Clash/Netlist/Util.hs:1713:13 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist.Util
      expandTopEntityOrErrM, called at src/Clash/Netlist/Util.hs:773:8 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist.Util
      mkUniqueNormalized, called at src/Clash/Netlist.hs:268:9 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist
      genComponentT, called at src/Clash/Netlist.hs:243:41 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist
      genComponent, called at src/Clash/Netlist.hs:122:9 in clash-lib-1.6.4-0fce510277c9374c65dcd33bbd3daf26e29cf24adf6075fb212c2a4232211e81:Clash.Netlist

Note how I both applied port annotation "A"and port annotation "B" to a tuple ("C" ::: Bool, "D" ::: Bool)

lmbollen avatar May 24 '23 12:05 lmbollen

This:

"A" ::: Signal System ("B" ::: ....

is pretty ambiguous. On the one hand I'd expect this to combine them into A_B, but on the other hand I'd expect B to simply replace A. IMO, the TH code should have returned an error.

martijnbastiaan avatar May 24 '23 12:05 martijnbastiaan

I don't think it's ambiguous. I feel it's morally the same as this.

data Foo a = Foo ("B" ::: a)

"A" ::: Signal System (Foo Int)

Which is a great way to nest these annotations.

rowanG077 avatar May 24 '23 12:05 rowanG077

I guess this is a result of allowing partial PortProduct annotations. e.g. the following is allowed:

t_output = PortProduct "Q" [PortName "A"]
topEntity :: Int -> (Int, Bool)
topEntity x = (x + 1, x == 0)

And the first result of the tuple will be called Q_A and the second part of the tuple will be called Q_AUTOGENERATED.

christiaanb avatar May 24 '23 12:05 christiaanb

I think the error message is misleading. I think the problem is that PortProduct only works on types with more than one "field". So when it does

PortProduct "A" [x]

it is setting itself up for failure. It is saying Bool is not a product type in the error message but this is misdirection, we're not looking at a Bool for x. x represents a (Bool, Bool), but it is the only one, so it only warrants a single PortProduct term not two nested ones.

The following works:

{-# ANN topEntity
   Synthesize{ t_name="topEntity"
             , t_inputs = []
             , t_output = PortProduct "A_B"
                            [ PortName "C"
                            , PortName "D"
                            ]
             } #-}

DigitalBrains1 avatar Feb 01 '24 15:02 DigitalBrains1