inlineCleanup can create empty letrecs

[martijnbastiaan]

Consider the following pseudo-code (due to GHC optimizations I can't write a small example that triggers this behavior):

f a = prim (let b = a in (b && False))

Thanks to #984 this will be turned into:

f a = prim (let b = a in False)

inlineCleanup will then remove the binder b, because it is unused:

f a = prim (let {} in False)

Leaving us with an EmptyRec. If this is forced in a blackbox, Clash will crash with Forced to evaluate unexpected function argument. For this particular case, we can patch inlineCleanup to omit a let if it has no bindings:

diff --git a/clash-lib/src/Clash/Normalize/Transformations.hs b/clash-lib/src/Clash/Normalize/Transformations.hs
index 2d24ab3b..5b6a2e3a 100644
--- a/clash-lib/src/Clash/Normalize/Transformations.hs
+++ b/clash-lib/src/Clash/Normalize/Transformations.hs
@@ -2297,8 +2297,10 @@ inlineCleanup (TransformContext is0 _) (Letrec binds body) = do
       bodyFVs       = Lens.foldMapOf freeLocalIds unitVarSet body
       (il,keep)     = List.partition (isInteresting allOccs prims bodyFVs) binds
       keep'         = inlineBndrs is1 keep il
-  if null il then return  (Letrec binds body)
-             else changed (Letrec keep' body)
+
+  if | null il -> return  (Letrec binds body)
+     | null keep' -> changed body
+     | otherwise -> changed (Letrec keep' body)

In general, I'm not sure what to do. Should we assign the letrec to a var before passing it to a blackbox?

[christiaanb]

Nah,

clash-compiler/clash-lib/src/Clash/Netlist/BlackBox.hs

Line 200 in 398c4ad

mkArgument bndr e = do

should just have an extra clause for let-expressions where it creates declarations for the let-bindings, and recursively call itself on the body of the let expression

[martijnbastiaan]

@christiaanb Is there a specific reason it wasn't accepting letrecs?

[christiaanb]

@martijnbastiaan legacy reasons, we used to be stricter in our normal forms. You will have to call mkUniqueNormalized on the let-expression though, in order to ensure that the String parts of the name (which is used for the HDL name) is unique. See the other uses of mkUniqueNormalized in BlackBox.hs