derivePackedAnnotation creates bogus bit representations

[DigitalBrains1]

It seems derivePackedAnnotation is somehow not encoding a constructor discriminator in some cases, picking the first value (all zeroes) for all constructors in that position. For instance, it seems that this datatype

data B
  = Ba Bit
  | Bb (BitVector 2)
  | Bc

should have led to Expected but actually results in Actual:

Term	Expected	Actual
Ba n	0b0n1	0b0n1
Bb n	0b1nn	0b0nn
Bc	0b0.0	0b0.0

The same issue exists with the example from our documentation(!). Also, it seems to be the same in Clash master and Clash 1.6.4.

Reproducer with a few bells and whistles:

{-# OPTIONS_GHC -Wno-missing-signatures #-}

module DeriveBitReprSmall where

import Clash.Prelude
import Clash.Annotations.BitRepresentation.Deriving

data A
  = Aa Bit
  | Ab (BitVector 2)
  | Ac
  deriving (Generic, Show, NFDataX, BitPack)

data B
  = Ba Bit
  | Bb (BitVector 2)
  | Bc
  deriving (Generic, Show, NFDataX, Eq)
derivePackedAnnotation [t| B |]
deriveBitPack [t| B |]

vecA = Aa 0 :> Aa 1 :> Ab 0 :> Ab 1 :> Ab 2 :> Ab 3 :> Ac :> Nil
vecB = Ba 0 :> Ba 1 :> Bb 0 :> Bb 1 :> Bb 2 :> Bb 3 :> Bc :> Nil

synA = (smap (flip const) vecA, smap (\_ x -> pack x) vecA)
{-# ANN synA (defSyn "synA") #-}

synB = (smap (flip const) vecB, smap (\_ x -> pack x) vecB)
{-# ANN synB (defSyn "synB") #-}

check = putStrLn $ foldr f "" $ zip vecB $ map (unpack . pack) vecB
 where
  f (x, y) s
    | x == y = s
    | otherwise = "Unequal: " <> sh x <> " /= " <> sh y <> "\n" <> s
  sh x0 = show x0 <> " (" <> show (pack x0) <> ")"

Some output:

>>> synB
(Ba 0 :> Ba 1 :> Bb 0b00 :> Bb 0b01 :> Bb 0b10 :> Bb 0b11 :> Bc :> Nil,
0b001 :> 0b011 :> 0b000 :> 0b001 :> 0b010 :> 0b011 :> 0b0.0 :> Nil)
>>> check
Unequal: Ba 0 (0b001) /= Bb 0b01 (0b001)
Unequal: Ba 1 (0b011) /= Bb 0b11 (0b011)
Unequal: Bc (0b0.0) /= Bb 0b.0 (0b0.0)

HDL behaves the same as Clash simulation, but mind the reverse when reading HDL :-).

A reproducer for Train from the documentation

{-# OPTIONS_GHC -Wno-missing-signatures #-}

module DeriveBitRepr where

import Clash.Prelude
import Clash.Annotations.BitRepresentation.Deriving

type SmallInt = Unsigned 2

data TrainA
   = PassengerA SmallInt
   | FreightA SmallInt SmallInt
   | MaintenanceA
   | ToyA
   deriving (Generic, Show, NFDataX, BitPack)

data TrainB
   = PassengerB SmallInt
   | FreightB SmallInt SmallInt
   | MaintenanceB
   | ToyB
   deriving (Generic, Show, NFDataX, BitPack)
derivePackedAnnotation [t| TrainB |]

data TrainC
   = PassengerC SmallInt
   | FreightC SmallInt SmallInt
   | MaintenanceC
   | ToyC
   deriving (Generic, Show, NFDataX)
derivePackedAnnotation [t| TrainC |]
deriveBitPack [t| TrainC |]

vecA =
     PassengerA 0
  :> PassengerA 1
  :> PassengerA 2
  :> PassengerA 3
  :> FreightA 0 0
  :> FreightA 0 1
  :> FreightA 0 2
  :> FreightA 0 3
  :> FreightA 1 0
  :> FreightA 3 3
  :> MaintenanceA
  :> ToyA
  :> Nil

vecB =
     PassengerB 0
  :> PassengerB 1
  :> PassengerB 2
  :> PassengerB 3
  :> FreightB 0 0
  :> FreightB 0 1
  :> FreightB 0 2
  :> FreightB 0 3
  :> FreightB 1 0
  :> FreightB 3 3
  :> MaintenanceB
  :> ToyB
  :> Nil

vecC =
     PassengerC 0
  :> PassengerC 1
  :> PassengerC 2
  :> PassengerC 3
  :> FreightC 0 0
  :> FreightC 0 1
  :> FreightC 0 2
  :> FreightC 0 3
  :> FreightC 1 0
  :> FreightC 3 3
  :> MaintenanceC
  :> ToyC
  :> Nil

varA = (smap (flip const) vecA, smap (\_ x -> pack x) vecA)
{-# ANN varA (defSyn "varA") #-}

varB = (smap (flip const) vecB, smap (\_ x -> pack x) vecB)
{-# ANN varB (defSyn "varB") #-}

varC = (smap (flip const) vecC, smap (\_ x -> pack x) vecC)
{-# ANN varC (defSyn "varC") #-}

That reproducer also investigates what happens when you omit deriveBitPack; I concluded the problem is purely in derivePackedAnnotation from the HDL generated for that.

[rowanG077]

I touched that code last.... it's probably me who broke it. Although not specifically the packed representation stuff.

Could you test with commit 17b9f807ee01c7ee7b5379ee5f6e9ff60aeab4a6? That's one commit prior to my changes.

[DigitalBrains1]

You can rest easy, that still fails in the exact same way :-)

[DigitalBrains1]

Bisecting this is no fun at all; I think just analysing the code is more productive. But I did find that it was already broken in the same way in 285ecef, which is from 29 July 2019. I didn't build older versions than that, so that commit is in no way related to the issue; I'm just pointing out it was already broken then.