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

Added the ~ISSCALAR template #2184

Merged
merged 2 commits into from
May 4, 2022
Merged

Added the ~ISSCALAR template #2184

merged 2 commits into from
May 4, 2022

Conversation

rowanG077
Copy link
Member

@rowanG077 rowanG077 commented May 2, 2022

Added the ~ISSCALAR template which can be used to check if an argument is rendered to a scalar in HDL.

@rowanG077 rowanG077 changed the title Added the ~ISSCALARTYPE template Added the ~ISSCALAR template May 2, 2022
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add it to this list?

* @~RESULT@: Signal to which the result of a primitive must be assigned
to. NB: Only used in a /declaration/ primitive.
* @~ARG[N]@: @(N+1)@'th argument to the function.
* @~CONST[N]@: @(N+1)@'th argument to the function. Like @~ARG@, but Clash will
try to reduce this to a literal, even if it would otherwise consider it too
expensive. And if Clash fails to reduce this argument to a literal it will
produce an error.
* @~LIT[N]@: @(N+1)@'th argument to the function. Like @~CONST~ but values are
rendered as a bare literals, without any size or type annotations.
This only works for numeric types, and not for BitVector.
* @~TYP[N]@: VHDL type of the @(N+1)@'th argument.
* @~TYPO@: VHDL type of the result.
* @~TYPM[N]@: VHDL type/name/ of the @(N+1)@'th argument; used in /type/
/qualification/.
* @~TYPMO@: VHDL type/name/ of the result; used in /type qualification/.
* @~ERROR[N]@: Error value for the VHDL type of the @(N+1)@'th argument.
* @~ERRORO@: Error value for the VHDL type of the result.
* @~GENSYM[\<NAME\>][N]@: Create a unique name, trying to stay as close to
the given @\<NAME\>@ as possible. This unique symbol can be referred to in
other places using @~SYM[N]@.
* @~SYM[N]@: a reference to the unique symbol created by @~GENSYM[\<NAME\>][N]@.
* @~SIGD[\<HOLE\>][N]@: Create a signal declaration, using @\<HOLE\>@ as the name
of the signal, and the type of the @(N+1)@'th argument.
* @~SIGDO[\<HOLE\>]@: Create a signal declaration, using @\<HOLE\>@ as the name
of the signal, and the type of the result.
* @~TYPEL[\<HOLE\>]@: The element type of the vector type represented by @\<HOLE\>@.
The content of @\<HOLE\>@ must either be: @~TYP[N]@, @~TYPO@, or @~TYPEL[\<HOLE\>]@.
* @~COMPNAME@: The name of the component in which the primitive is instantiated.
* @~LENGTH[\<HOLE\>]@: The vector length of the type represented by @\<HOLE\>@.
* @~DEPTH[\<HOLE\>]@: The tree depth of the type represented by @\<HOLE\>@.
The content of @\<HOLE\>@ must either be: @~TYP[N]@, @~TYPO@, or @~TYPEL[\<HOLE\>]@.
* @~SIZE[\<HOLE\>]@: The number of bits needed to encode the type represented by @\<HOLE\>@.
The content of @\<HOLE\>@ must either be: @~TYP[N]@, @~TYPO@, or @~TYPEL[\<HOLE\>]@.
* @~IF \<CONDITION\> ~THEN \<THEN\> ~ELSE \<ELSE\> ~FI@: renders the @\<ELSE\>@
part when @\<CONDITION\>@ evaluates to /0/, and renders the @\<THEN\>@ in all
other cases. Valid @\<CONDITION\>@s are @~LENGTH[\<HOLE\>]@, @~SIZE[\<HOLE\>]@,
@~DEPTH[\<HOLE\>]@, @~VIVADO@, @~IW64@, @~ISLIT[N]@, @~ISVAR[N]@, @~ISACTIVEENABLE[N]@,
@~ISSYNC[N]@, and @~AND[\<HOLE1\>,\<HOLE2\>,..]@.
* @~VIVADO@: /1/ when Clash compiler is invoked with the @-fclash-xilinx@ or
@-fclash-vivado@ flag. To be used with in an @~IF .. ~THEN .. ~ELSE .. ~FI@
statement.
* @~TOBV[\<HOLE\>][\<TYPE\>]@: create conversion code that so that the
expression in @\<HOLE\>@ is converted to a bit vector (@std_logic_vector@).
The @\<TYPE\>@ hole indicates the type of the expression and must be either
@~TYP[N]@, @~TYPO@, or @~TYPEL[\<HOLE\>]@.
* @~FROMBV[\<HOLE\>][\<TYPE\>]@: create conversion code that so that the
expression in @\<HOLE\>@, which has a bit vector (@std_logic_vector@) type,
is converted to type indicated by @\<TYPE\>@. The @\<TYPE\>@ hole must be
either @~TYP[N]@, @~TYPO@, or @~TYPEL[\<HOLE\>]@.
* @~INCLUDENAME[N]@: the generated name of the @N@'th included component.
* @~FILEPATH[\<HOLE\>]@: The argument mentioned in @\<HOLE\>@ is a file which
must be copied to the location of the generated HDL.
* @~GENERATE@: Verilog: create a /generate/ statement, except when already in
a /generate/ context.
* @~ENDGENERATE@: Verilog: create an /endgenerate/ statement, except when already
in a /generate/ context.
* @~ISLIT[N]@: Is the @(N+1)@'th argument to the function a literal.
* @~ISVAR[N]@: Is the @(N+1)@'th argument to the function explicitly not a
literal.
* @~TAG[N]@: Name of given domain. Errors when called on an argument which is not
a 'KnownDomain', 'Reset', or 'Clock'.
* @~PERIOD[N]@: Clock period of given domain. Errors when called on an argument
which is not a 'KnownDomain' or 'KnownConf'.
* @~ISACTIVEENABLE[N]@: Is the @(N+1)@'th argument a an Enable line NOT set to a
constant True. Can be used instead of deprecated (and removed) template tag
@~ISGATED@. Errors when called on an argument which is not a signal of bools.
* @~ISSYNC[N]@: Does synthesis domain at the @(N+1)@'th argument have synchronous resets. Errors
when called on an argument which is not a 'KnownDomain' or 'KnownConf'.
* @~ISINITDEFINED[N]@: Does synthesis domain at the @(N+1)@'th argument have defined initial
values. Errors when called on an argument which is not a 'KnownDomain' or 'KnownConf'.
* @~ACTIVEEDGE[edge][N]@: Does synthesis domain at the @(N+1)@'th argument respond to
/edge/. /edge/ must be one of 'Falling' or 'Rising'. Errors when called on an
argument which is not a 'KnownDomain' or 'KnownConf'.
* @~AND[\<HOLE1\>,\<HOLE2\>,..]@: Logically /and/ the conditions in the @\<HOLE\>@'s
* @~VAR[\<NAME\>][N]@: Like @~ARG[N]@ but binds the argument to a variable named NAME.
* @~VARS[N]@: VHDL: Return the variables at the @(N+1)@'th argument.
* @~NAME[N]@: Render the @(N+1)@'th string literal argument as an identifier
instead of a string literal. Fails when the @(N+1)@'th argument is not a
string literal.
* @~DEVNULL[\<HOLE\>]@: Render all dependencies of @\<HOLE\>@, but disregard direct output.
* @~REPEAT[\<HOLE\>][N]@: Repeat literal value of @\<HOLE\>@ a total of @N@ times.
* @~TEMPLATE[\<HOLE1\>][\<HOLE2\>]@: Render a file @\<HOLE1\>@ with contents @\<HOLE2\>@.

@rowanG077
Copy link
Member Author

@martijnbastiaan Fixed

@alex-mckenna
Copy link
Contributor

You should also note this is incomplete wrt some HDL. For instance, VHDL specifies that integers, reals and user-defined enums are also scalar types. Verilog treats integer and real as vector types (insofar as you can bit-select and part-select into them). By not including them we implicitly bias towards the verilog view of the world, but maybe we should take the more restrictive view and treat them as scalar in Clash?

This isn't something to solve now, but it should be documented

Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with @alex-mckenna

@alex-mckenna
Copy link
Contributor

You're also missing copyright notices 👀

@rowanG077 rowanG077 force-pushed the ISSCALARTYPE branch 3 times, most recently from ce6b0f9 to 5350ff3 Compare May 2, 2022 09:09
@rowanG077 rowanG077 force-pushed the ISSCALARTYPE branch 3 times, most recently from f6701e5 to e211936 Compare May 2, 2022 11:32
@@ -476,6 +492,12 @@ renderElem b (IF c t f) = do
Literal {} -> 1
BlackBoxE {} -> 1
_ -> 0
(IsScalar n) -> let (_,ty,_) = bbInputs b !! n
in case ty of
-- See NOTE [scalar netlist types]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of this function is:

renderElem
  :: HasCallStack
  => Backend backend
  => BlackBoxContext
  -> Element
  -> State backend (Int -> Text)

i.e. we get access to the Backend backend => backend. This means we get access to

hdlKind :: state -> HDL
which allows us to differentiate between the HDL we're rendering for. This in turns means we can decide what is a scalar or not depending on the hdl we're generating, obviating the need for the note, and just implementing it properly.

Copy link
Member Author

@rowanG077 rowanG077 May 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christiaanb I have implemented it. I'm no Verilog/VHDL star but from the specs I can read online in (System)Verilog only Bit and Bool are scalar. in VHDL in addition to those integer, real, enumeration and physical types are scalar. But we never generate real or physical types.

@rowanG077 rowanG077 force-pushed the ISSCALARTYPE branch 4 times, most recently from bb2ddba to 64310a5 Compare May 2, 2022 15:43
@rowanG077
Copy link
Member Author

I noticed the inputHole function which is used to validate a blackbox is missing quite a few cases. Is this by design? Or just forgotten to update over the years?

inputHole :: Element -> Maybe Int
inputHole = \case
Arg n -> pure n
Lit n -> pure n
Const n -> pure n
Name n -> pure n
Typ (Just n) -> pure n
TypM (Just n) -> pure n
Err (Just n) -> pure n
_ -> Nothing

Copy link
Contributor

@alex-mckenna alex-mckenna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good now I think, modulo that one nit-picking comment 😉

clash-lib/src/Clash/Netlist/BlackBox/Util.hs Outdated Show resolved Hide resolved
@alex-mckenna alex-mckenna merged commit 7843abe into master May 4, 2022
@alex-mckenna alex-mckenna deleted the ISSCALARTYPE branch May 4, 2022 07:20
mergify bot pushed a commit that referenced this pull request May 4, 2022
* Added `~ISSCALAR` template

* FIXED: Input validation of the used arguments in blackboxes is now complete.

(cherry picked from commit 7843abe)
alex-mckenna pushed a commit that referenced this pull request May 4, 2022
* Added `~ISSCALAR` template

* FIXED: Input validation of the used arguments in blackboxes is now complete.

(cherry picked from commit 7843abe)

Co-authored-by: Rowan Goemans <goemansrowan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants