Currently, copilot-bluespec–generated structs are given Bits instances, but nothing else. I propose that we also define FShow instances as well. These are very useful for printing out values in Bluespec applications, and we would be able to make use of these instances in the copilot-bluespec test suite itself.

Currently, copilot-bluespec has a fair bit of code that is copy-pasted from copilot-c99:

• Almost all of the code in Copilot.Compile.Bluespec.External is taken directly from Copilot.Compile.C99.External.

• This code in Copilot.Compile.Bluespec.Name:

  copilot-bluespec/src/Copilot/Compile/Bluespec/Name.hs

  Lines 69 to 95 in 9d83645

  	-- | Turn a stream id into a suitable Bluespec variable name.
  	streamName :: Id -> String
  	streamName sId = "s" ++ show sId
  	-- | Turn a stream id into the global varname for indices.
  	indexName :: Id -> String
  	indexName sId = streamName sId ++ "_idx"
  	-- | Turn a stream id into the name of its accessor function
  	streamAccessorName :: Id -> String
  	streamAccessorName sId = streamName sId ++ "_get"
  	-- | Turn stream id into name of its generator function.
  	generatorName :: Id -> String
  	generatorName sId = streamName sId ++ "_gen"
  	-- | Turn the name of a trigger into a guard generator.
  	guardName :: String -> String
  	guardName name = lowercaseName name ++ "_guard"
  	-- | Turn a trigger name into a an trigger argument name.
  	argName :: String -> Int -> String
  	argName name n = lowercaseName name ++ "_arg" ++ show n
  	-- | Enumerate all argument names based on trigger name.
  	argNames :: String -> [String]
  	argNames base = map (argName base) [0..]

  Is taken from Copilot.Compile.C99.Name.

• This code in Copilot.Compile.Bluespec.Compile:

  copilot-bluespec/src/Copilot/Compile/Bluespec/Compile.hs

  Lines 262 to 294 in 9d83645

  	-- ** Obtain information from Copilot Core Exprs and Types.
  	-- | List all types of an expression, returns items uniquely.
  	exprTypes :: Typeable a => Expr a -> [UType]
  	exprTypes e = case e of
  	Const ty _ -> typeTypes ty
  	Local ty1 ty2 _ e1 e2 -> typeTypes ty1 `union` typeTypes ty2
  	`union` exprTypes e1 `union` exprTypes e2
  	Var ty _ -> typeTypes ty
  	Drop ty _ _ -> typeTypes ty
  	ExternVar ty _ _ -> typeTypes ty
  	Op1 _ e1 -> exprTypes e1
  	Op2 _ e1 e2 -> exprTypes e1 `union` exprTypes e2
  	Op3 _ e1 e2 e3 -> exprTypes e1 `union` exprTypes e2
  	`union` exprTypes e3
  	Label ty _ _ -> typeTypes ty
  	-- | List all types of a type, returns items uniquely.
  	typeTypes :: Typeable a => Type a -> [UType]
  	typeTypes ty = case ty of
  	Array ty' -> typeTypes ty' `union` [UType ty]
  	Struct x -> concatMap (\(Value ty' _) -> typeTypes ty') (toValues x)
  	`union` [UType ty]
  	_ -> [UType ty]
  	-- | Collect all expression of a list of streams and triggers and wrap them
  	-- into an UEXpr.
  	gatherExprs :: [Stream] -> [Trigger] -> [UExpr]
  	gatherExprs streams triggers = map streamUExpr streams
  	++ concatMap triggerUExpr triggers
  	where
  	streamUExpr (Stream _ _ expr ty) = UExpr ty expr
  	triggerUExpr (Trigger _ guard args) = UExpr Bool guard : args

  Is taken from Copilot.Compile.C99.Compile.

• Much of the infrastructure in the unit tests in each package.

Ideally, we could find some way to share this code in between the two libraries to avoid code duplication. One complication is that most of this code lives in modules that aren't exported. Perhaps it would make sense to create a new copilot-backend library that factors out the code shared in common?

Currently, BluespecSettings only has a single option that can be configured:

We may want to consider other options to add here. For example, do we want Bluespec equivalents of copilot-c99's cSettingsStepFunctionName? There are several function names that copilot-bluespec generates, and it might make sense to allow users to be able to change them.

copilot-3.20 adds support for struct updates via the new UpdateField operation. We should add support for translating UpdateField operations into Bluespec's functional updates (e.g., v { x = 27; y = 42 }).

This release will include support for Copilot struct updates (#10) and building against copilot-core-3.20.

Currently, the copilot-bluespec README says:

This is not actually true, however, as copilot-bluespec is not currently distributed with the main copilot library. Instead, we should direct users to install copilot-bluespec on its own using cabal.

Now that copilot-core-3.19 is released to Hackage, we should:

1. Require that as the minimum version in copilot-bluespec.cabal,
2. Bump copilot-bluespec's version to 3.19, and
3. Release it to Hackage

copilot-bluespec incorrectly translates the Copilot signum function to Bluespec.

signum @Int8

In Copilot, signum @Int8 0 should return 0. In Bluespec, however, it returns 1:

package Top where

mkTop :: Module Empty
mkTop =
  module
    rules
      "step": when True ==> do
        let x :: Int 8
            x = 0
        $display "%d\n" (signum x)
        $finish

$ bsc -sim -g mkTop -u Top.bs
checking package dependencies
compiling Top.bs
code generation for mkTop starts
Elaborated module file created: mkTop.ba
All packages are up to date.

$ bsc -sim -e mkTop -o mkTop
Bluesim object created: mkTop.{h,o}
Bluesim object created: model_mkTop.{h,o}
Simulation shared library created: mkTop.so
Simulation executable created: mkTop

$ ./mkTop
   1

signum @Double

In Copilot, signum @Double x returns x whenever x is 0.0, -0.0, or NaN. In Bluespec, however, it will return 1.0 if the sign bit is clear and -1.0 if the sign bit is set. Here are some examples demonstrating this in action:

package Top where

import FloatingPoint

mkTop :: Module Empty
mkTop =
  module
    rules
      "step": when True ==> do
        let x1 :: Double
            x1 = 0.0
        $display "%x\n" (signum x1)
        let x2 :: Double
            x2 = negate 0.0
        $display "%x\n" (signum x2)
        let x3 :: Double
            x3 = qnan
        $display "%x\n" (signum x3)
        let x4 :: Double
            x4 = negate qnan
        $display "%x\n" (signum x4)
        $finish

$ bsc -sim -g mkTop -u Top.bs
checking package dependencies
compiling Top.bs
code generation for mkTop starts
Elaborated module file created: mkTop.ba
All packages are up to date.

$ bsc -sim -e mkTop -o mkTop
Bluesim object created: mkTop.{h,o}
Bluesim object created: model_mkTop.{h,o}
Simulation shared library created: mkTop.so
Simulation executable created: mkTop

$ ./mkTop
3ff0000000000000

bff0000000000000

3ff0000000000000

bff0000000000000

(Note that 0x3ff0000000000000 is the hexadecimal representation of 1.0 :: Double as bits, and 0xbff0000000000000 is the hexadecimal representation of -1.0 :: Double as bits.)

Proposed fix

In order to make these translations faithful to Copilot's semantics, I think we will need to translate signum e to something like if e > 0 then 1 else (if e < 0 then -1 else e) in Bluespec. This is similar to how copilot-c99 translates signum (see this code).

(Spun off from a discussion in #14 (comment).)

This Copilot specification:

-- Bug.hs
{-# LANGUAGE NoImplicitPrelude #-}
module Main (main) where

import Copilot.Compile.Bluespec
import Language.Copilot

spec :: Spec
spec = trigger "trig" (constD 0 < extern "e" Nothing) []

main :: IO ()
main = do
  spec' <- reify spec
  compile "Bug" spec'

Will result in Bluespec code that fails to compile against this Top.bs file:

-- Top.bs
package Top where

import FloatingPoint

import Bug
import BugIfc
import BugTypes

bugIfc :: Module BugIfc
bugIfc =
  module
    e_impl :: Reg Double <- mkReg 0
    interface
      e = e_impl
      trig =
        $display "Hello, World!"

mkTop :: Module Empty
mkTop = mkBug bugIfc

$ runghc Bug.hs
$ bsc -sim -g mkTop -u Top.bs
checking package dependencies
compiling Top.bs
Warning: "Top.bs", line 11, column 4: (T0054)
  Field not defined: `e'
code generation for mkTop starts
Error: "FloatingPoint.bsv", line 948, column 86: (S0015)
  Bluespec evaluation-time error: Unordered comparison of type
  `FloatingPoint'.
  During elaboration of the explicit condition of rule `trig' at "Bug.bs",
  line 18, column 8.
  During elaboration of `mkTop' at "Top.bs", line 15, column 0.

The same problem arises if you use (<=), (>), or (>=) instead of (<).

The culprit is this part of the generated Bug.bs file:

mkBug :: Prelude.Module BugIfc -> Prelude.Module Prelude.Empty;
mkBug ifcMod = 
    module {
      ifc <- ifcMod;
      let { trig_guard :: Prelude.Bool;
            trig_guard =  (Prelude.<) (0.0::Double) ifc.e._read; };
      rules {
        
        "trig":  when trig_guard   ==> ifc.trig
      }
    }

Here, we are using Bluespec's (<) function to compare the value of ifc.e._read, a Double value that comes from a register. This is bad, as Bluespec's (<) function is partial and will error on NaN values, resulting in the evaulation-time error seen above. See B-Lang-org/bsc#711 for more information on why this happens.

The solution is to not translate the Copilot expression x < y to the Bluespec expression x < y. Instead, we should translate it to something like compareFP x y == LT (where compareFP is defined here). Unlike (<) and friends, compareFP is total and will not error if one of the inputs is a NaN value.