Skip to content

Commit c0a0d03

Browse files
Fix dubious warnings
1 parent 04e7ba0 commit c0a0d03

33 files changed

Lines changed: 500 additions & 73 deletions

HACKING.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,33 @@ a big win. You need about 2GB of RAM for each thread.
135135
cabal run clash-testsuite -- --auto-detect-tools -j$(nproc)
136136
```
137137

138+
#### Turning Clash warnings into test failures: `-fclash-werror`
139+
140+
Some compiler bugs only show up as a warning on stderr, which by itself does not
141+
fail a test. `-fclash-werror` makes Clash throw a `ClashException` instead of
142+
printing a warning, so a test that adds it to its `clashFlags` fails as soon as
143+
Clash starts warning:
144+
145+
```haskell
146+
runTest "PopCountNoInteger" def{clashFlags=["-fclash-werror"]}
147+
```
148+
149+
Unlike GHC's `-Werror`, `-fclash-werror` only affects Clash's own warnings
150+
(GHC's `-Werror` also sets it). Warnings this covers include:
151+
152+
* `Dubious primitive instantiation ...` — e.g. an `Integer` reaching the
153+
netlist. Useful for pinning down code that should be `Integer`-free.
154+
* `Unmatchable constant as case subject ...` — a constant the primitive
155+
evaluator has no reduction rule for, so `caseCon` cannot fold the
156+
case-expression. Only reported when invariants are checked, which
157+
[`commonArgs`](tests/src/Test/Tasty/Clash.hs) already does for every test via
158+
`-fclash-debug DebugSilent`.
159+
160+
Note that this cannot be turned on testsuite-wide: a number of existing tests
161+
legitimately provoke these warnings (`Numbers/Bits` exercises `Bits Integer`,
162+
and `TopEntity/T1074` has a type-equality dictionary as a case subject, which is
163+
benign but irreducible). Enable it per test instead.
164+
138165
#### `-p` (pattern filter)
139166

140167
`-p PATTERN` is a [tasty pattern][tasty-patterns] that restricts which tests
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FIXED: `(++#)` on `BitVector`s was only constant folded when *both* operands were literals. A zero-width operand is often not a literal but `removedArg` (`reduceSplitHandler` produces one for `split#` with a zero-width half), so the evaluator got stuck. In a constant case subject this made `caseCon` report `Unmatchable constant as case subject` and left a redundant multiplexer in the generated HDL. A zero-width operand no longer has to be a literal.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FIXED: `GHC.Num.Integer.integerLogBase#` and `GHC.Num.Natural.naturalLogBase#` were not constant folded outside the domain of `Clash.Util.flogBase` (`y <= 0`), even though GHC's `integerLogBase#` is total. Such calls appear in (dead) `KnownNat` evidence for ill-defined applications of e.g. `GHC.TypeLits.Extra.CLog`, and produced an `Unmatchable constant as case subject` report. Clash now mirrors the values GHC computes at runtime.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FIXED: `popCount`, `countLeadingZeros` and `countTrailingZeros` on `BitVector` (and hence on `Unsigned`, `Signed` and `Index`, which delegate to it) went through `Integer`, which made Clash emit a `Dubious primitive instantiation for GHC.Num.Integer.integerToInt#` warning and put an `Integer` in the generated HDL. They now use `Clash.Sized.Internal.Index.fromEnum#`, which has a black box of its own.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ADDED: `-fclash-werror` turns Clash's own warnings into errors. Unlike GHC's `-Werror` it does not make GHC's warnings fatal, which makes it usable in tests: a test that should not provoke any Clash warning can pass `-fclash-werror` and will fail as soon as it does.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CHANGED: The `Unmatchable constant as case subject` report (emitted when invariants are checked, e.g. under `-fclash-debug DebugSilent`) is now a proper warning instead of a trace, so `-Werror` / `-fclash-werror` turns it into an error. It also names the binder being normalized.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CHANGED: `Dubious primitive instantiation` warnings now name the component, the binder the primitive's result is assigned to, and the primitive's arguments (resolving variable references against sibling let-bindings). They are also deduplicated per *(component, primitive)* pair instead of per primitive, so a single run reveals every offending site rather than only the first. Designs with many offending components therefore emit more warnings than before.

clash-ghc/src-bin-9.10.1/Clash/Main.hs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,10 @@ main' postLoadMode units dflags0 args flagWarnings startAction clashOpts = do
271271
-- Propagate some GHC flags to Clash
272272
liftIO . modifyIORef' clashOpts $ \opts ->
273273
opts
274-
{ opt_werror = EnumSet.member Opt_WarnIsError (generalFlags dflags3)
274+
-- Note the '||': -fclash-werror has already been parsed at this point,
275+
-- and GHC's -Werror should not undo it.
276+
{ opt_werror = opt_werror opts
277+
|| EnumSet.member Opt_WarnIsError (generalFlags dflags3)
275278
, opt_ghcDebugLevel = debugLevel dflags3
276279
}
277280

clash-ghc/src-bin-9.10.2/Clash/Main.hs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,10 @@ main' postLoadMode units dflags0 args flagWarnings startAction clashOpts = do
271271
-- Propagate some GHC flags to Clash
272272
liftIO . modifyIORef' clashOpts $ \opts ->
273273
opts
274-
{ opt_werror = EnumSet.member Opt_WarnIsError (generalFlags dflags3)
274+
-- Note the '||': -fclash-werror has already been parsed at this point,
275+
-- and GHC's -Werror should not undo it.
276+
{ opt_werror = opt_werror opts
277+
|| EnumSet.member Opt_WarnIsError (generalFlags dflags3)
275278
, opt_ghcDebugLevel = debugLevel dflags3
276279
}
277280

clash-ghc/src-bin-9.12/Clash/Main.hs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,10 @@ main' postLoadMode units dflags0 args flagWarnings startAction clashOpts = do
287287

288288
-- Propagate -Werror to Clash
289289
liftIO . modifyIORef' clashOpts $ \opts ->
290-
opts { opt_werror = EnumSet.member Opt_WarnIsError (generalFlags dflags3) }
290+
-- Note the '||': -fclash-werror has already been parsed at this point, and
291+
-- GHC's -Werror should not undo it.
292+
opts { opt_werror = opt_werror opts
293+
|| EnumSet.member Opt_WarnIsError (generalFlags dflags3) }
291294

292295
let dflags4 = if backendNeedsFullWays bcknd &&
293296
not (gopt Opt_ExternalInterpreter dflags3)

0 commit comments

Comments
 (0)