| Age | Commit message (Collapse) | Author |
|
* [WIP] Propagate source locators to Verilog if-else emission
* Add and fix tests for reg update info propagation
* Add limited source locator propagation in ConstProp
Support propagating source locators on connections or nodes where the
right-hand side is simply a reference. This case comes up a lot for
registers without a synchronous reset.
node _T_1 = x @[MyFile.scala 12:10]
node _T_2 = _T_1
z <= x
Previousy the source locator would be lost, now the result is:
z <= x @[MyFile.scala 12:10]
* Address review comments
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Refactor the test used in the CustomTransformSpec to assert that
inputForm=LowForm legacy transforms run right before the emitter (see
note below!). The new test looks only for a list of (customTransform,
emitter) in a sliding, size-2 window of the flattened transform order.
Previously, this was looking for a match before and after the custom
transform. The old implementation necessitate busywork updates of the
test when new transforms are added that changed the transform running
before the custom transform.
Note: this test, as written is intentionally wrong. When verification
statements were added, the test was changed to not do what it's
supposed to do. Namely, the test is supposed to ensure that an
inputForm=LowForm transform runs immediately before its emitter.
However, the test is actually checking that the custom transform runs
before transforms that convert and remove verification statements. I'm
intentionally leaving the test broken, but doing the refactor in order
to make this easier to manually backport to the 1.3.x branch.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Remove tests from LoweringCompilerSpec testing the placement of
inputForm=LowForm legacy, custom transforms. This behavior is already
tested in the CustomTransformSpec.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
This should speed up the common case
as the compiler never operates on the
unescaped string.
The new escape function also fixes a bug
where ']' was not escaped even though it
is the delimiting character for FileInfo.
In order to maintain backwards
compatibility for the ProtoBuf format,
this patch adds escape/unescape calls
when going from/to protobuf format.
For better performance we should consider
changing the protobuf format.
|
|
Add an optionalPrerequisiteOf to TopWiringTransform pointing at
LowerTypes.
This fixes a bug where top-wired wire bundles with flips could result
in the top-wired, flattened bundle having every field with an "output"
direction if the TopWiringTransform is moved around in the transform
order (see FIRRTL issue #1744).
Why did this happen?
Fundamentally, this stems from the fact that LowerTypes preserves
bundle direction for ports, but destroys it for wires.
Specifically, The TopWiringTransform creates ports of the "output"
direction that are copies of the underlying type of the component
being top-wired. Before LowerTypes, the type of a bundle has direction
information via flips. After LowerTypes, the lowered ground type does
not have this information. Therefore, all the ports are ground type
outputs. Simply ensuring that TopWiringTransform must run before
LowerTypes avoids this problem.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
|
|
For multi-line registers, the parsed source locator is located in a
different place in the concrete syntax tree than it is for other
Statements.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
* RemoveAccess: update gender to flow
* ExpandWhens: update female to sink
* RemoveCHIRRTL: female -> sink, male -> source
|
|
* benchmark: add hot.DedupBenchmark
* dedup: use structural md5 hash instead of agnostify and serialize
* StructuralHash: generate PrimOp LUT
* StructuralHash: change MessageDigestHasher to not be a case class
* StructuralHash: we want Blocks and EmptyStmt to be ignored
* StructuralHash: use SHA-256 instead of MD5
* StructuralHash: clarify extmodule port name agnistification
* StructuralHash: hash the name of width vars instead of trying to agnostify
This should be in line with the old Dedup behavior.
The prior use of n(..) was incorrect since the namespace
of these vars is different from the normal module scope namespace.
* StructuralHash: address Schuyler's review comments
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
This Serializer which is implemented
external to the IR node definition
uses a StringBuilder to achieve about a
1.7x performance improvement when serializing.
Eventually, all implementations of the
`serialize` methd should be replaced with
a call to `Serializer.serialize`.
However, for this PR we keep the old
code in place in order to allow for easy
regression testing with the benchmark JAR
like this:
> java -cp utils/bin/firrtl-benchmark.jar \
firrtl.benchmark.hot.SerializationBenchmark \
~/benchmarks/medium.pb 2 5 test
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Fix a bug where a type check would always yield true. This caused a
bug where allow/block-list annotations would be incorrectly applied to
all subtypes of ManipulateNames.
The tests are updated to check that this now works.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
|
|
* Add testcase for empty message
|
|
* ConvertAsserts maps each assert into a gated print-and-stop
* ConvertAsserts is an optional prereq of RemoveVerificationStatements
* ConvertAsserts generates Low FIRRTL
* Drop print for asserts that have an empty message
* Fix scaladoc formatting from review
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
* Batch renames in LowerTypes
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
* Add benchmarking for LowerTypes
Add infrastructure of benchmarking Transforms (in addition to
existing infra for Passes). Also run System.gc between each timed
benchmark to improve stability.
Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Adds an options to the FIRRTL compiler command line to schedule the
LowerCaseNames and UpperCaseNames transforms.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
This adds three new transforms:
- (abstract) LetterCaseTransform parent of case manipulation
- LowerCaseNames to lower case all names
- UpperCaseNames to upper case all names
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Add tests for the ManipulateNames transform.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Add a new annotation that stores the resulting name of an allowlist
name to be manipulated.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Rewrite of RemoveKeywordCollisions into a more generic
ManipulateNames. The new ManipulateNames transform is abstract in a
'(String, Namespace) => String' method that can be used for arbitrary
manipulation of names in a circuit. The software architecture remains
mostly the same (a rename map is used as the underlying data store).
However, the new ManipulateNames used Target as opposed to Named.
Add the ability for naming to be selectively enabled or disabled via:
- ManipulateNamesAllowlistAnnotation
- ManipulateNamesBlocklistAnnotation
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
message (#1717)
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
If a module has ports of type Bundle that are used in aggregate
connections in parent modules, Dedup cannot change the names of the
fields of the Bundle or it would change the semantics of the connection.
Dedup now detects this case and refrains from agnostifying the ports of
such modules to prevent this issue.
|
|
* Add assume, assert, cover statements
* Assert submodule assumptions
* Add warning when removing verification statements
* Remove System Verilog behaviour emitter warning
* Add option to disable AssertSubmoduleAssumptions
* Document verification statements in the spec
The syntax for the new statements is
assert(clk, cond, en, msg)
assume(clk, cond, en, msg)
cover(clk, cond, en, msg)
With assert as a representative example, the semantics is as follows:
`clk` is the clock, `cond` is the expression being asserted, `en` is the
enable signal (if `en` is low then the assert is not checked) and `msg`
is a string message intended to be reported as an error message by the
model checker if the assertion fails.
In the Verilog emitter, the new statements are handled by a new
`formals` map, which groups the statements by clock domain. All model
checking statements are then emitted within the context of an `ifdef
FORMAL` block, which allows model checking tools (like Symbiyosys) to
utilize the statements while keeping them out of synthesis flows.
Co-authored-by: Albert Magyar <albert.magyar@gmail.com>
|
|
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
* Support Memory Initialization for Simulation and FPGA Flows
This adds a minimal annotation that allows users to
influence if memories are randomly initialized,
if all entries are initialized to the same scalar or
if each entry should be initialized to a different value.
We use the init block in order to initialize memories
which is supported by verilator as well as yosys
and has previously been used to randomize the initial
memory contents.
* LowerTypes: error when trying to split up a memory with MemoryInitAnnotation
Currently the MemoryInitAnnotation only works for
ground-type memories.
We catch misuse of this annotation at the point of
the firrtl compiler at which memories on non-ground type
get split up, i.e., the LowerTypes transform.
Chisel should try to prevent annotating non-ground
type memories in the frontend, but it is nice to
have an additional check.
* MemoryInitSpec: test JSON deserialization
* MemoryInitAnnotation: split up into three different annotations instead of exposing MemoryInitValue
Co-authored-by: Albert Magyar <albert.magyar@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
* Add attributes, ifdefs to emitter.
* Make ifdef API a little cleaner.
* Remove references to ifdefs.
* Remove more of the ifdef stuff I missed
* Fix up failing tests
* Add multiple attribute test case
* Remove tpe as a parameter from Annotations.
Some general refactoring.
* Add some documentation.
* Incorporate some feedback
* Expand some spaghetti code, add comments
* Fix type signature by removing it
* bug fix in test
* Fix unchecked type parameter matches in AddDescriptionNodes.
* use target to replace name
Co-authored-by: Paul Rigge <rigge@berkeley.edu>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
|
|
* delete usages of toSet for determinism
* add formatting suggestion from code review
|
|
This message informs the user, it does not indicate
a failure.
|
|
* Build ArrayBuffers in Block.mapStmt
* Have empty Block serialize as "skip"
The FIRRTL parser requires at least one indented line in each module.
Sometimes tests emit and parse modules with no contents; this ensures
there's always at least a "skip" in empty modules.
Also fix tests that expected certain skips
* Use var List as stack in Block.mapStmt impl
This replaces Iterator concatenation. In Scala 2.11, RHS recursion on
Iterators is not stack safe. This seems to have been fixed in 2.12 by
Scala PR 5033.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Use WrappedExpression instead of MemoizedHash. The benefit of memoizing
the hash pales in comparison to the cost of hashing deeply nested Types
in the AST.
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
|
|
|
|
* Switch to more idiomatic flatMap for Utils.expandRef
|
|
* Avoid excessively deep recursion
* Avoid overhead of DFS for shallow expression trees
* Reduce work: skip expressions that cannot contain error-containing subtrees
* Review feedback: added commentary to explain new check_widths_e structure
|
|
{Lower,Upper}CaseNames Transforms (#1651)
* Revert "Add test of {Lower, Upper}CaseNames"
This reverts commit 93c078b8469bc55cd2d33147c33e2b5421fda9d9.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
* Revert "Add --change-name-case <lower|upper> option"
This reverts commit d3ab7e2db66fe3a63778f427dad6c08f64695ba5.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
* Revert "Add features.{LowerCaseNames, UpperCaseNames} transforms"
This reverts commit c8dcdacf313f19a4d0238be694478a325432edd4.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
* Revert "Refactor RemoveKeywordCollisions->ManipulateNames"
This reverts commit c534c5abae7b80a725ec9925569b3383b3c24a34.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
|
|
|
* This will help encourage use of these rather than WIR
|
|
Fix a performance bug in DedupModules introduced in #1539. Stop
recalculating the same expensive datastructures for each module,
potentially multiple times.
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
|
- RenameMap Behavior
-- Prevent transitive renaming A -> B -> C (continueRenaming)
-- Prevent transitive renaming for self-renames
- Target
-- Override toString as serialize for CompleteTarget
-- Expansion of stripHierarchy to enable stripping InstanceTargets to become ModuleTargets
Annotations
-- Bugfix in extractComponents where Products were not iterated over
-- Converts renamed targets to local targets using Target.referringModule to preserve sticky behavior
- Eliminate Target Paths
-- Make DuplicationHelper use LinkedHashMap, as we iterate over its contents and convert to Seq in def makePathless
-- Add DupedResult to map original module to new module targets
-- Update renaming to record a map from all relative instance paths to original module, to new module target
-- Consumes DedupedResult to give better name to new duplicated module if it was originally deduplicated
-- Reorder modules in attempt to preserve original ordering, pre-deduplication
-- Move utility functions to object
-- Bugfix: add self-renames to prevent ofModule _ of target _ cannot be renamed to Vector(_, _, _, ...) errors
- Dedup
-- Changed NoDedupAnnotation to contain ModuleTarget, rather than ModuleName
-- Added DedupedResult to map original module to the duplicate module
-- Consumes DupedResult to pick better name, if it existed
-- Updates renaming to chain the following: instancify deduped modules, remap differently named internal signals, then remap AST modules
-- Move utility functions to object
-- Remove annotations as part of determination of dedup correctness
-- Bugfix: add instance renames so that deduped modules have their instances properly renamed
- Dead Code Elimination
-- Add deletion of ASTModules
- Tests
-- Morphism Spec to ensure Dedup -> EliminateTargetPaths and EliminateTargetPaths -> Dedup patterns work properly
-- Update existing tests to make sure they work properly
-- Add Dedup tests to demonstrate instance renaming bug, EliminateTargetPaths for ofModule rename bug, and update RenameMap tests
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: Adam Izraelevitz <adam.izraelevitz@sifive.com>
Co-authored-by: Adam Izraelevitz <azidar@gmail.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
|
|
Change WrappedTransforms to be sensitive to the --class-log-level of
their true underlying transform. In effect, information logged in a
wrapper (like timing information) will now print as expected.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|