From 68b6dc7e408d71c4edb77f9ad073e4d0371fe70d Mon Sep 17 00:00:00 2001 From: Christiaan Baaij Date: Mon, 18 Nov 2024 13:53:24 +0100 Subject: [PATCH] Do not inline let-bound recursive calls This prevents `recToLetRec` from turning a globally recursive function into a local let-bound recursive function which can be synthesized to hardware. Fixes #2839 (cherry picked from commit 00722413f0e2da5fcab5c87be7f083d5585099dc) --- changelog/2024-11-18T13_43_58+01_00_fix2839 | 1 + .../src/Clash/Normalize/Transformations/Inline.hs | 9 ++++++++- tests/Main.hs | 1 + tests/shouldwork/Issues/T2839.hs | 12 ++++++++++++ 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 changelog/2024-11-18T13_43_58+01_00_fix2839 create mode 100644 tests/shouldwork/Issues/T2839.hs diff --git a/changelog/2024-11-18T13_43_58+01_00_fix2839 b/changelog/2024-11-18T13_43_58+01_00_fix2839 new file mode 100644 index 0000000000..568318c964 --- /dev/null +++ b/changelog/2024-11-18T13_43_58+01_00_fix2839 @@ -0,0 +1 @@ +FIXED: Clash errored saying it cannot translate a globally recursive function in code that originally only contains let-bound (local) recursion [#2839](https://github.com/clash-lang/clash-compiler/issues/2839). diff --git a/clash-lib/src/Clash/Normalize/Transformations/Inline.hs b/clash-lib/src/Clash/Normalize/Transformations/Inline.hs index d74a7070dd..5443307eda 100644 --- a/clash-lib/src/Clash/Normalize/Transformations/Inline.hs +++ b/clash-lib/src/Clash/Normalize/Transformations/Inline.hs @@ -120,7 +120,14 @@ bindConstantVar = inlineBinders test True -> return (i `notElemFreeVars` e) _ -> do tcm <- Lens.view tcCache - case isWorkFreeIsh tcm e of + (fn,_) <- Lens.use curFun + -- Don't inline things that perform work, it increases the circuit size. + -- + -- Also don't inline globally recursive calls, it prevents the + -- recToLetRec transformation from transforming global recursion to + -- local recursion. + -- See https://github.com/clash-lang/clash-compiler/issues/2839 + case isWorkFreeIsh tcm e && not (e == Var fn) of True -> Lens.view inlineConstantLimit >>= \case 0 -> return True n -> return (termSize e <= n) diff --git a/tests/Main.hs b/tests/Main.hs index dba10fa892..e5db4706aa 100755 --- a/tests/Main.hs +++ b/tests/Main.hs @@ -791,6 +791,7 @@ runClashTest = defaultMain $ clashTestRoot , runTest "T2623CaseConFVs" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]} , runTest "T2781" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]} , runTest "T2628" def{hdlTargets=[VHDL], buildTargets=BuildSpecific ["TACacheServerStep"], hdlSim=[]} + , runTest "T2839" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]} ] <> if compiledWith == Cabal then -- This tests fails without environment files present, which are only diff --git a/tests/shouldwork/Issues/T2839.hs b/tests/shouldwork/Issues/T2839.hs new file mode 100644 index 0000000000..6a95bb1e72 --- /dev/null +++ b/tests/shouldwork/Issues/T2839.hs @@ -0,0 +1,12 @@ +module T2839 where + +import Clash.Explicit.Prelude +import Clash.Explicit.Testbench + +topEntity :: + Signal System (Unsigned 8) +topEntity = register clk noReset enableGen 100 0 + where + cntr = register clk noReset enableGen (0 :: Unsigned 8) 0 + done = (== 100) <$> cntr + clk = tbClockGen $ not <$> done