clash-lang / clash-compiler

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

nameHint is unreliable due to foldr/build fusion #2794

Open bgamari opened 2 months ago

bgamari commented 2 months ago

If one writes nameHint (SSym @"hi") ... there is a good chance that GHC's unpack rewrite rule will rewrite the String in the term-level evidence carried by SSym (which is the only thing preserved in the netlist) into build (unpackFoldrCString# "hi"). The logic in Clash.Netlist.BlackBox.Util.exprToString understandably cannot deal with this, resulting in errors of the form:

    Clash error call:
    Clash.Netlist.BlackBox.Util(761): Error when reducing to string in ~NAME construct: Clash.Netlist.BlackBox.Util(937): Could not extract string from BlackBoxE "Clash.Promoted.Symbol.SSymbol" [] [] [] (BBTemplate [Lit 0]) (Context {bbName = "Clash.Promoted.Symbol.SSymbol", bbResults = [(Identifier (RawIdentifier "v" Nothing []) Nothing,String)], bbInputs = [(BlackBoxE "GHC.CString.unpackFoldrCString#" [] [] [] (BBTemplate [Lit 0]) (Context {bbName = "GHC.CString.unpackFoldrCString#", bbResults = [(Identifier (RawIdentifier "v" Nothing []) Nothing,String)], bbInputs = [(Literal Nothing (StringLit "mealyState"),String,True),(Identifier (RawIdentifier "Clash.Normalize.Primitives.removedArg" Nothing []) Nothing,Void Nothing,False),(BlackBoxE "Clash.Normalize.Primitives.removedArg" [] [] [] (BBTemplate [Err Nothing]) (Context {bbName = "Clash.Normalize.Primitives.removedArg", bbResults = [(Identifier (RawIdentifier "v" Nothing []) Nothing,String)], bbInputs = [], bbFunctions = fromList [], bbQsysIncName = [], bbLevel = 0, bbCompName = RawIdentifier "clk_manager" (Just (UniqueIdentifier {i_baseName = "clk_manager", i_baseNameCaseFold = "clk_manager", i_extensionsRev = [], i_idType = Basic, i_hdl = Verilog, i_provenance = []})) [], bbCtxName = Just "$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep11_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep10_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep9_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep1_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep_build_v"}) True,String,True)], bbFunctions = fromList [(1,[(Right (RawIdentifier "" Nothing [],[Assignment (RawIdentifier "~RESULT" Nothing []) Cont (Identifier (RawIdentifier "~ERRORO" Nothing []) Nothing)]),Cont,[],[],[],Context {bbName = "Clash.Normalize.Primitives.removedArg", bbResults = [(Identifier (RawIdentifier "v" Nothing []) Nothing,String)], bbInputs = [], bbFunctions = fromList [], bbQsysIncName = [], bbLevel = 1, bbCompName = RawIdentifier "clk_manager" (Just (UniqueIdentifier {i_baseName = "clk_manager", i_baseNameCaseFold = "clk_manager", i_extensionsRev = [], i_idType = Basic, i_hdl = Verilog, i_provenance = []})) [], bbCtxName = Just "$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep11_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep10_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep9_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep1_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep_build_v"})])], bbQsysIncName = [], bbLevel = 0, bbCompName = RawIdentifier "clk_manager" (Just (UniqueIdentifier {i_baseName = "clk_manager", i_baseNameCaseFold = "clk_manager", i_extensionsRev = [], i_idType = Basic, i_hdl = Verilog, i_provenance = []})) [], bbCtxName = Just "$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep11_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep10_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep9_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep1_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep_build_v"}) True,String,True)], bbFunctions = fromList [], bbQsysIncName = [], bbLevel = 0, bbCompName = RawIdentifier "clk_manager" (Just (UniqueIdentifier {i_baseName = "clk_manager", i_baseNameCaseFold = "clk_manager", i_extensionsRev = [], i_idType = Basic, i_hdl = Verilog, i_provenance = []})) [], bbCtxName = Just "$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep11_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep10_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep9_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep1_$s$fHasEntityPortsFUN_$centityPortRep_$s$w$centityPortRep_v"}) True referred to by Lit 0

Perplexingly, the build call doesn't seem to be reflected in the netlist.

bgamari commented 2 months ago

It is not entirely clear what to do about this. Ideally we would be able to disable the rewrite into foldr/build form, but sadly GHC isn't really equipped for this sort of selective disabling of rules. The obvious alternative is to perform the inverse rewrite in Clash, although this could get tricky.

christiaanb commented 2 months ago

We should add an primitive evaluator rule for unpackFoldrCString# to https://github.com/clash-lang/clash-compiler/blob/b14ff0ef2ccfad8854210a9035e9db1e32b3be07/clash-ghc/src-ghc/Clash/GHC/Evaluator/Primitive.hs#L236

bgamari commented 2 months ago

I suppose this rule should rewrite unpackFoldrCString# "..."# (:) [] to "..."?

christiaanb commented 2 months ago

Yes, preferably rewrite it to unpackCString# “…” instead of (:) '.' ((:) '.' ((:) '.' [])). Although it’s probably good to always have the latter translation as a fallback in case the function and the starting value are something else than (:) and []