From 52272d2c84d4184797e17bbc20f2906fa22be625 Mon Sep 17 00:00:00 2001 From: Leon Schoorl Date: Thu, 20 Jun 2024 17:00:09 +0200 Subject: [PATCH 1/2] Fix the test for #1796 Something probably changed since it was introduced. It would prefer to read the source instead of the precompiled module and always succeed, even with the original fix reverted. (cherry picked from commit a40ff920dc44dd06c12e71e631f7b56aa5e8ec5a) --- tests/clash-testsuite.cabal | 6 ++--- tests/shouldwork/LoadModules/T1796.hs | 27 ++----------------- .../LoadModules/precompiled/T1796a.hs | 15 +++++++++++ 3 files changed, 20 insertions(+), 28 deletions(-) create mode 100644 tests/shouldwork/LoadModules/precompiled/T1796a.hs diff --git a/tests/clash-testsuite.cabal b/tests/clash-testsuite.cabal index dd0e43324b..2748d06909 100644 --- a/tests/clash-testsuite.cabal +++ b/tests/clash-testsuite.cabal @@ -102,7 +102,7 @@ library -- Behaviour when loading modules can differ if the module is loaded from -- an external interface file. See -- https://github.com/clash-lang/clash-compiler/issues/1796 for an example. - shouldwork/LoadModules + shouldwork/LoadModules/precompiled exposed-modules: Test.Tasty.Common @@ -118,8 +118,8 @@ library Test.Tasty.Vivado Test.Tasty.Clash.CollectSimResults - -- From tests/shouldwork/LoadModules - T1796 + -- From tests/shouldwork/LoadModules/precompiled + T1796a other-modules: Paths_clash_testsuite diff --git a/tests/shouldwork/LoadModules/T1796.hs b/tests/shouldwork/LoadModules/T1796.hs index f7421da925..81f0a921bc 100644 --- a/tests/shouldwork/LoadModules/T1796.hs +++ b/tests/shouldwork/LoadModules/T1796.hs @@ -4,29 +4,6 @@ module T1796 where -import Clash.Prelude +import T1796a -import Clash.Explicit.Testbench - -topEntity - :: Signal System Bool - -> Signal System Bool -topEntity = id --- See: https://github.com/clash-lang/clash-compiler/pull/2511 -{-# CLASH_OPAQUE topEntity #-} - -tb - :: Signal System Bool -tb = done - where - testInput = stimuliGenerator clk rst (singleton True) - expectOutput = outputVerifier' clk rst (singleton True) - done = expectOutput $ topEntity testInput - clk = tbClockGen @System (not <$> done) - rst = resetGen @System -{-# INLINE tb #-} - -testBench :: Signal System Bool -testBench = tb --- See: https://github.com/clash-lang/clash-compiler/pull/2511 -{-# CLASH_OPAQUE testBench #-} +topEntity = tb diff --git a/tests/shouldwork/LoadModules/precompiled/T1796a.hs b/tests/shouldwork/LoadModules/precompiled/T1796a.hs new file mode 100644 index 0000000000..ce67bdcf13 --- /dev/null +++ b/tests/shouldwork/LoadModules/precompiled/T1796a.hs @@ -0,0 +1,15 @@ +{-# LANGUAGE CPP #-} +{-# LANGUAGE NoImplicitPrelude #-} +{-# LANGUAGE TypeApplications #-} + +module T1796a where + +import Clash.Explicit.Prelude +import Clash.Explicit.Testbench + +tb :: Signal System Bool +tb = done + where + done = register clk rst enableGen False $ id (pure True) + clk = tbClockGen @System (not <$> done) + rst = resetGen @System From b305d8d9bf452b9ea04ffad7e44e0b522ab5dd52 Mon Sep 17 00:00:00 2001 From: Leon Schoorl Date: Thu, 20 Jun 2024 16:57:55 +0200 Subject: [PATCH 2/2] Make sure clashSimulation is False in HDL even with -O2 The SpecConstr pass in GHC is able to poke through the OPAQUE of clashSimulation. Adding a noinline seems to block it from seeing the True inside. Fixes #2736 (cherry picked from commit 68fb3da233858e8b403e8b2be4db640216b4af8c) --- clash-prelude/src/Clash/Magic.hs | 4 +++- tests/Main.hs | 2 +- tests/clash-testsuite.cabal | 4 ++++ .../Basic/{SimulationMagic.hs => SimulationMagic2736.hs} | 9 +++++---- .../shouldwork/Basic/precompiled/SimulationMagic2736a.hs | 8 ++++++++ 5 files changed, 21 insertions(+), 6 deletions(-) rename tests/shouldwork/Basic/{SimulationMagic.hs => SimulationMagic2736.hs} (85%) create mode 100644 tests/shouldwork/Basic/precompiled/SimulationMagic2736a.hs diff --git a/clash-prelude/src/Clash/Magic.hs b/clash-prelude/src/Clash/Magic.hs index 4df15eed52..0de40d0afd 100644 --- a/clash-prelude/src/Clash/Magic.hs +++ b/clash-prelude/src/Clash/Magic.hs @@ -44,6 +44,7 @@ module Clash.Magic ) where import Data.String.Interpolate (__i) +import GHC.Magic (noinline) import GHC.Stack (HasCallStack, withFrozenCallStack) import Clash.NamedTypes ((:::)) import GHC.TypeLits (Nat,Symbol) @@ -259,7 +260,8 @@ noDeDup = id -- | 'True' in Haskell/Clash simulation. Replaced by 'False' when generating HDL. clashSimulation :: Bool -clashSimulation = True +clashSimulation = noinline True +-- The 'noinline' is here to prevent SpecConstr from poking through the OPAQUE, see #2736 -- See: https://github.com/clash-lang/clash-compiler/pull/2511 {-# CLASH_OPAQUE clashSimulation #-} diff --git a/tests/Main.hs b/tests/Main.hs index 8b626c2931..2cc4fd2de7 100755 --- a/tests/Main.hs +++ b/tests/Main.hs @@ -385,7 +385,7 @@ runClashTest = defaultMain $ clashTestRoot , runTest "NameInstance" def{hdlSim=[]} , outputTest "NameInstance" def , outputTest "SetName" def{hdlTargets=[VHDL]} - , outputTest "SimulationMagic" def{hdlTargets=[VHDL]} + , outputTest "SimulationMagic2736" def{hdlTargets=[VHDL]} , runTest "PatError" def{hdlSim=[]} , runTest "ByteSwap32" def , runTest "CharTest" def diff --git a/tests/clash-testsuite.cabal b/tests/clash-testsuite.cabal index 2748d06909..d5c1543c6c 100644 --- a/tests/clash-testsuite.cabal +++ b/tests/clash-testsuite.cabal @@ -103,6 +103,7 @@ library -- an external interface file. See -- https://github.com/clash-lang/clash-compiler/issues/1796 for an example. shouldwork/LoadModules/precompiled + shouldwork/Basic/precompiled exposed-modules: Test.Tasty.Common @@ -121,6 +122,9 @@ library -- From tests/shouldwork/LoadModules/precompiled T1796a + -- From tests/shouldwork/Basic/precompiled + SimulationMagic2736a + other-modules: Paths_clash_testsuite diff --git a/tests/shouldwork/Basic/SimulationMagic.hs b/tests/shouldwork/Basic/SimulationMagic2736.hs similarity index 85% rename from tests/shouldwork/Basic/SimulationMagic.hs rename to tests/shouldwork/Basic/SimulationMagic2736.hs index 3d992d1566..3cf4608510 100644 --- a/tests/shouldwork/Basic/SimulationMagic.hs +++ b/tests/shouldwork/Basic/SimulationMagic2736.hs @@ -1,4 +1,6 @@ -module SimulationMagic where +{-# LANGUAGE TemplateHaskellQuotes #-} +{-# OPTIONS_GHC -O2 -fspec-constr #-} +module SimulationMagic2736 where import Prelude as P import Data.List (isInfixOf) @@ -6,11 +8,10 @@ import System.Environment (getArgs) import System.FilePath (()) import Clash.Prelude -import Clash.Magic (clashSimulation) +import SimulationMagic2736a f :: Int -f | clashSimulation = 123 - | otherwise = 456 +f = f0 {-# ANN f (defSyn "f") #-} assertNotIn :: String -> String -> IO () diff --git a/tests/shouldwork/Basic/precompiled/SimulationMagic2736a.hs b/tests/shouldwork/Basic/precompiled/SimulationMagic2736a.hs new file mode 100644 index 0000000000..090e227591 --- /dev/null +++ b/tests/shouldwork/Basic/precompiled/SimulationMagic2736a.hs @@ -0,0 +1,8 @@ +{-# OPTIONS_GHC -O2 -fspec-constr #-} +module SimulationMagic2736a where + +import Clash.Prelude + +f0 :: Int +f0 | clashSimulation = 123 + | otherwise = 456