search

clash-lang / clash-compiler

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

Exception: CLaSH.Netlist(179): Not in normal form: RHS of case-projection is not a variable #111

Closed ggreif closed 8 years ago

ggreif commented 8 years ago

Attached CAM.txt exposes another bug (?) in the VHDL generator:

*CAM> :vhdl
[1 of 1] Compiling CAM              ( CAM.hs, CAM.o )
Loading dependencies took 0.46461s
Applied 365 transformations
Normalisation took 0.803514s
*** Exception: CLaSH.Netlist(179): Not in normal form: RHS of case-projection is not a variable: case CLaSH.Prelude.BlockRam.blockRam#
     @(CLaSH.Signal.Internal.Clk system 1000)
     @(GHC.TypeLits.^ 2 6)
     @(CAM.Bucket GHC.Types.Bool GHC.Types.Bool)
       64
       CLaSH.Signal.Explicit.systemClock1913188173
       (CLaSH.Sized.Vector.replicate
        @(GHC.TypeLits.^ 2 6)
        @(CLaSH.Sized.Vector.Vec
            2 (GHC.Base.Maybe (GHC.Tuple.(,) GHC.Types.Bool GHC.Types.Bool)))
          (CLaSH.Promoted.Nat.SNat
           @(GHC.TypeLits.^ 2 6)
             64
             (Data.Proxy.Proxy
              @GHC.TypeLits.Nat
              @(GHC.TypeLits.^ 2 6)))
          (CLaSH.Sized.Vector.replicate
           @2
           @(GHC.Base.Maybe (GHC.Tuple.(,) GHC.Types.Bool GHC.Types.Bool))
             (CLaSH.Promoted.Nat.SNat
              @2
                2
                (Data.Proxy.Proxy
                 @GHC.TypeLits.Nat
                 @2))
             (GHC.Base.Nothing
              @(GHC.Tuple.(,) GHC.Types.Bool GHC.Types.Bool))))
       bodyVar_61189
       bodyVar_91186
       eta_i31179
       eta_i41180 of
  CLaSH.Signal.Internal.:-
    (ww1 :: CAM.Bucket GHC.Types.Bool GHC.Types.Bool)
    (ww2 :: CLaSH.Signal.Internal.Signal'
              (CLaSH.Signal.Internal.Clk system 1000)
              (CAM.Bucket GHC.Types.Bool GHC.Types.Bool)) ->
    GHC.Prim.(#,#)
    @(CAM.Bucket GHC.Types.Bool GHC.Types.Bool)
    @(CLaSH.Signal.Internal.Signal'
        (CLaSH.Signal.Internal.Clk system 1000)
        (CAM.Bucket GHC.Types.Bool GHC.Types.Bool))
      ww1
      ww2

Changing readNew to id fixes the generation problem, but of course, the semantics suffer. B.t.w. this passes the Haskell test.

Also I manually defunctionalised the code (relative to issue #109) so that __VOID__ shouldn't appear in the generated vhdl (using id instead of readNew). Vivado is happy too.

christiaanb commented 8 years ago

Given that the constructor of the Signal type is exposed, I'm afraid it's related to the annoying CPR problem described here: https://github.com/clash-lang/clash-compiler/blob/master/clash-ghc/src-ghc/CLaSH/GHC/LoadModules.hs#L288

To fix this, I need to figure out why GHC is doing the worker/wrapper and CPR analysis, so I can fix the specific module in the prelude. Perhaps, CLaSH.Prelude.BlockRam needs to be compiled with -O0.

Edit: or compile CLaSH.Prelude.BlockRam with -fno-strictness.

ggreif commented 8 years ago

Then this is possibly not a bug at all, as I am compiling with 7.11.20150407, which did not have your CPR suppression patch yet.

Em terça-feira, 12 de janeiro de 2016, Christiaan Baaij < notifications@github.com> escreveu:

Given that the constructor of the Signal type is exposed, I'm afraid it's related to the annoying CPR problem described here: https://github.com/clash-lang/clash-compiler/blob/master/clash-ghc/src-ghc/CLaSH/GHC/LoadModules.hs#L288

To fix this, I need to figure out why GHC is doing the worker/wrapper and CPR analysis, so I can fix the specific module in the prelude. Perhaps, CLaSH.Prelude.BlockRam needs to be compiled with -O0.

— Reply to this email directly or view it on GitHub https://github.com/clash-lang/clash-compiler/issues/111#issuecomment-171066982 .

ggreif commented 8 years ago

@christiaanb that said, I am not sure QC works with v8.0, so I am wary to upgrade. Also I have no 7.10.x at work :-(

ggreif commented 8 years ago

@christiaanb forgot to ask, can you repro it?

christiaanb commented 8 years ago

My internet at home is broken it seems, and my phone/provider doesn't allow tethering, so I won't be able to test it until I get to the office tomorrow morning.

ggreif commented 8 years ago

@christiaanb no urgency at all 😀

christiaanb commented 8 years ago

It works on my machine (using GHC 7.10.2). The output of the VHDL simulator is:

./../../src/ieee/numeric_std-body.v93:1613:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:1157:7:@0ms:(assertion warning): NUMERIC_STD."<": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1157:7:@0ms:(assertion warning): NUMERIC_STD."<": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:1613:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1613:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:1613:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1613:7:@0ms:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@0ms:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
CAM2_outputVerifier_zdsoutputVerifierzm_10.vhdl:71:9:@2ns:(assertion error): outputVerifier, expected: XX, actual: 00
CAM2_outputVerifier_zdsoutputVerifierzm_10.vhdl:71:9:@3ns:(assertion error): outputVerifier, expected: XX, actual: 00
../../../src/ieee/numeric_std-body.v93:1613:7:@2003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@2003ns:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:1613:7:@2003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1613:7:@4003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1613:7:@7003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:2124:7:@7003ns:(assertion warning): NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0
../../../src/ieee/numeric_std-body.v93:1613:7:@8003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE
../../../src/ieee/numeric_std-body.v93:1613:7:@10003ns:(assertion warning): NUMERIC_STD."=": metavalue detected, returning FALSE

meaning the generated VHDL is correct. Well, mostly... it seems that the circuits is outputting 0 instead of the expected X when it comes out of reset. But that's OK I guess.

christiaanb commented 8 years ago

Don't close this bug yet though, we might still need to add a {-# OPTIONS_GHC -fno-cpr-anal #-} pragma to the CLaSH.Prelude.BlockRam when compiling with a newer version of ghc HEAD.

ggreif commented 8 years ago

Any ETA for clash in conjunction with GHC v8.0 (or HEAD)?

christiaanb commented 8 years ago

Sadly not, it depends on when the dependencies of the compiler and the prelude are ready. Especially the singletons library and its dependencies: they use TH, and as those of us who run HEAD often know, TH code breaks on almost every minor release of GHC.

christiaanb commented 8 years ago

I have hacked patches for most dependencies on my own machine to test clash on ghc8, but they are probably out of sync. And, they are hacked, so not in any shape to be merged with the official repo, let alone be put on hackage.

ggreif commented 8 years ago

@christiaanb Okay I tried something different. Rebuilt clash-prelude with cabal install clash-prelude/ --ghc-option=-fcpr-off (after thoroughly cleaning it). Did the same to the other packages too. Then generated :vhdl. This time I got:

*CAM> :vhdl
[1 of 1] Compiling CAM              ( CAM.hs, CAM.o )
Loading dependencies took 0.294112s
Applied 361 transformations
Normalisation took 1.138219s
*** Exception: CLaSH.Netlist(124): CLaSH.Netlist.Util(53): Not in normal from: no Letrec:
 (eta :: CLaSH.Signal.Internal.Signal'
           (CLaSH.Signal.Internal.Clk system 1000)
           (CAM.Out GHC.Types.Bool GHC.Types.Bool)) ->
( a ->
  b ->
  clk ->
  (f :: CLaSH.Signal.Internal.Signal' clk a
        -> CLaSH.Signal.Internal.Signal' clk b) ->
  (x :: CLaSH.Signal.Internal.Signal' clk a) ->
 f x)
@(CAM.Out GHC.Types.Bool GHC.Types.Bool)
@(CAM.Func (CAM.Out GHC.Types.Bool GHC.Types.Bool))
@(CLaSH.Signal.Internal.Clk system 1000)
  ( (ds :: CAM.Out GHC.Types.Bool GHC.Types.Bool) ->
   case ds of
     CAM.None ->
        (ds1 :: CLaSH.Sized.Vector.Vec
                  2
                  (GHC.Base.Maybe (GHC.Tuple.(,) GHC.Types.Bool GHC.Types.Bool))) ->
       GHC.Base.Nothing
       @GHC.Types.Bool
     CAM.FindCol
       (k :: GHC.Types.Bool) ->
        (w2 :: CLaSH.Sized.Vector.Vec
                 (GHC.TypeLits.+ 1 1)
                 (GHC.Base.Maybe (GHC.Tuple.(,) GHC.Types.Bool GHC.Types.Bool))) ->
       CAM.$wa21912645335
       @GHC.Types.Bool
       @GHC.Types.Bool
         GHC.Classes.$fEqBool1912645324
         k
         w2)
  eta
Which has type:
CLaSH.Signal.Internal.Signal'
  (CLaSH.Signal.Internal.Clk system 1000)
  (CAM.Out GHC.Types.Bool GHC.Types.Bool)
-> CLaSH.Signal.Internal.Signal'
     (CLaSH.Signal.Internal.Clk system 1000)
     (CAM.Func (CAM.Out GHC.Types.Bool GHC.Types.Bool))

Did I miss something? Interestingly, replacing readNew with id still causes this new error, so I definitely made some progress.

This is the new file: CAM.txt It appears that the problem is related to the generic instance definition, having

instance BlockRamAssoc Bool Bool -- USING THIS INSTANCE (instead of the next) WORKS!
-- instance (Eq k, Eq v) => BlockRamAssoc k v

makes the error go away. There is something fishy with Classes.$fEqBool dictionary elimination?

This should probably be a new issue, as the CPR problem seems to be solved.

christiaanb commented 8 years ago

I'm observing the same thing. What's weirder is that commenting out the expectedOutput function makes the design compile. I don't know yet what's going on here.

christiaanb commented 8 years ago

OK, I know what's going on here. It's a combination of:

So yeah, definitely a bug, slight variations of GHC's Core for the same Haskell descriptions shouldn't influence whether CLaSH can generate VHDL/Verilog or not. I will fix.

christiaanb commented 8 years ago

As suggested by you already, can you perhaps file this as a separate bug report?

ggreif commented 8 years ago

Sure.

ggreif commented 8 years ago

Ooops there was still a "don't close yet" comment from you...

christiaanb commented 8 years ago

Both CAM.hs files compile correctly on GHC8-rc3 (+https://phabricator.haskell.org/D2108)