-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add entries for default parameters, and how to present parameters as strings. #19
Conversation
200533b
to
4197aa6
Compare
4197aa6
to
0af617b
Compare
@@ -590,4 +591,47 @@ struct Device { | |||
# Which sites have LUT BELs? | |||
lutElements @1 : List(LutElements); | |||
} | |||
|
|||
enum ParameterFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what ParameterFormat
would be used for a Bitstring
type parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ParameterFormat
is only used for textValue
, see comment here: https://github.com/SymbiFlow/fpga-interchange-schema/pull/19/files#diff-2cf4384b8130624e91f06b247760e831cb73d3362b117b7f3f1082721398c67eR615-R616
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like it might also be useful to specify that a parameter has to use Bitstring
instead of a textValue
representation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think tools should always accept textValue
representations, because all parameters can be encoded as a textValue
. Encoding as a more specific field type should be allowed, especially as an interior/intermediate state. For example, nextpnr should accept textValue
for LUT init, but for larger designs it is likely faster for nextpnr to be given bitstringValue
representation to avoid re-parsing constants.
Vendor tools (thinking Lattice and Vivado) will likely want textValue
-like representations, and so ParameterFormat
is more about narrowing the textValue
space for tools that accept narrower inputs. The bridge (e.g. PhysicalNetlistToDcp
) between the PhysicalNetlist
format and the vendor format (e.g. DCP) will be responsible for converting from the wider input space to the narrower vendor accepted space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, that seems reasonable
…strings. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>
0af617b
to
4ecd8b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #12 (from a schema perspective).