Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clash doesn't respect NOINLINE on polyvariadic functions with "extra" arguments #2502

Closed
martijnbastiaan opened this issue Jun 15, 2023 · 2 comments · Fixed by #2507
Closed
Assignees
Labels

Comments

@martijnbastiaan
Copy link
Member

Just going by the title I'm tempted to mark this as easy ;-).

Reproducer

import Data.String.Interpolate (__i)
import Clash.Explicit.Prelude
import Clash.Annotations.Primitive

class X a where
  x :: a

instance X (Signal System Int) where
  x = pure 3

instance X a => X (Signal System Int -> a) where
  x !_i = x

bb :: X a => Int -> a
bb !_ = x
{-# NOINLINE bb #-}
{-# ANN bb hasBlackBox #-}
{-# ANN bb (
  let bbName = show 'bb
  in InlineYamlPrimitive [minBound..] [__i|
    BlackBox:
      name: "#{bbName}"
      kind: Expression
      template: "~ARG[2]"
|]) #-}

bbWrapper :: X a => Int -> a
bbWrapper i = bb i
{-# NOINLINE bbWrapper #-}

topEntity :: Int -> Signal System Int -> Signal System Int
topEntity i0 i1 = bbWrapper i0 i1

Gives:

$ ls vhdl/Main.topEntity/
clash-manifest.json  Main_topEntity_types.vhdl  topEntity.vhdl

bbWrapper did not get its own file. DebugApplied log.

Removing the "extra" argument

import Data.String.Interpolate (__i)
import Clash.Explicit.Prelude
import Clash.Annotations.Primitive

class X a where
  x :: a

instance X (Signal System Int) where
  x = pure 3

instance X a => X (Signal System Int -> a) where
  x !_i = x

bb :: X a => a
bb = x
{-# NOINLINE bb #-}
{-# ANN bb hasBlackBox #-}
{-# ANN bb (
  let bbName = show 'bb
  in InlineYamlPrimitive [minBound..] [__i|
    BlackBox:
      name: "#{bbName}"
      kind: Expression
      template: "~ARG[1]"
|]) #-}

bbWrapper :: X a => a
bbWrapper = bb
{-# NOINLINE bbWrapper #-}

topEntity :: Signal System Int -> Signal System Int
topEntity i1 = bbWrapper i1

Gives:

$ ls vhdl/Main.topEntity/
bb1.vhdl  clash-manifest.json  Main_topEntity_types.vhdl  topEntity.vhdl

bbWrapper did get its own file. DebugApplied log.

Removing the polyvariadic aspect

import Data.String.Interpolate (__i)
import Clash.Explicit.Prelude
import Clash.Annotations.Primitive

bb :: Int -> Signal System Int -> Signal System Int
bb !_ !_ = pure 3
{-# NOINLINE bb #-}
{-# ANN bb hasBlackBox #-}
{-# ANN bb (
  let bbName = show 'bb
  in InlineYamlPrimitive [minBound..] [__i|
    BlackBox:
      name: "#{bbName}"
      kind: Expression
      template: "~ARG[1]"
|]) #-}

bbWrapper :: Int -> Signal System Int -> Signal System Int
bbWrapper i = bb i
{-# NOINLINE bbWrapper #-}

topEntity :: Int -> Signal System Int -> Signal System Int
topEntity i0 i1 = bbWrapper i0 i1

Gives:

$ ls vhdl/Main.topEntity/
bbWrapper.vhdl  clash-manifest.json  Main_topEntity_types.vhdl  topEntity.vhdl

bbWrapper did get its own file. DebugApplied log.

@christiaanb
Copy link
Member

Easy when you know where to look:

return (g,maybe inl bindingSpec gTmM, maybe specArg (Left . (`mkApps` gArgs) . bindingTerm) gTmM)

This doesn’t deal well with the case where the argument we specialize on lacks a NOINLINE spec; in two ways:

  1. It doesn’t preserve the NOINLINE spec of the function being specialized
  2. It doesn’t pick the name of the function being specialized when it has a NOINLINE, while the argument being specialized on does not

@christiaanb
Copy link
Member

We've released v1.8.0, which includes a fix for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants