Adapt `ExtractInstances` such that it can properly handle the new flavor
of NLAs, where non-local annotations would carry a symbol to a hierpath
that generally only points at their parent module.
The `ExtractInstances` pass had a few assumptions in its implementation
about hierpaths pointing at the annotation target op, and each anno
having a unique hierpath op associated with it. The implementation can
be simplified quite a bit once support for the old-style NLAs can be
ripped out.
This commit also adds explicit tests to check that the old *and* new
style of NLAs are working in parallel.
A nasty corner case still exists when extraction requires an NLA to be
re-routed because the instance has been extracted past the NLA's
original root. If the instance is multiply-extracted, one hierpath gets
turned into multiple hierpaths, which will require special handling of
*everything* with that hierpath. This commit detects the common case
which we can handle and ensures that the hierpath is re-rooted in place,
and produces a warning about a potential breakage of hierpath references
in the corner case.
This commit changes a default value of namekind from
interesting name to droppable name.
This is more comfortable in the situation where we want
to create a dummy unnamed wire for example
`builder.create<WireOp>(result.type())`. Previously this dummy wire
had an interesting name because default value is interesting.
This commit also changes IR representation accordingly.
This reverts commit db637d06b7. Since spillWiresForNamehints will not be used immediately at SiFIve, it would be saner to revert this to reduce the complexity of ExportVerilog.
Make the presence of an Annotation block the removal of a component.
The presence of an Annotation indicates that some later pass cares about
this wire and it cannot be safely deleted. Concretely, this enables
downstream passes from NOT having to slap DontTouchAnnotations on things
they care about (which implies, much strong, optimization block
behavior).
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
A situation can occur where a named declaration has its name re-named,
in which case the hw.verilogName is used to indicate the new
name. However, it is possible for collectNames to see a certain
expression that it wants to emit into a temporary, and decides to give
it a generated name that collides with another declaration's generated
hw.verilogName. This can happen if the expression is visited before
the declaration, and results in the declaration receiving a different
unique name, which doesn't match the assigned hw.verilogName.
This isn't a huge problem for local emission, but non-local things
like binds have to be able to trust the hw.verilogName attribute
holds the final name.
The change in this patch splits collectNames to loop over the IR
twice. First collecting names from declarations that may have a
hw.verilogName, and then collecting the rest of the names, and
doing the rest of the work like measuring widths, marking values
to emit, etc.
Some implementation alternatives considered:
* Compute some sort of data structure containing the names. We're
moving away from global name data structures in favor of using the
IR. Computing something locally would need a pass over the IR
anyway, so which would trade memory for runtime.
* Modify the hw.verilogName attr to match what ExportVerilog
selects. We are actively trying to make ExportVerilog mutate the IR
less, and make hw.verlogName more stable, and this would be the
opposite.
Name removal is no longer a thing we need to do, droppable names subsumes that. Further, there is an infinite application loop of NodeBypass which is fixed.
This commit fixes a (potential) bug that namehints of variadic ops are
copied to all lowered sub expressions. Copying sv.namehint to all sub
expression is weird even though it doesn't cause any bug
(since namehints created here are not used at all).
Update the InjectDUTHierarchy pass to work for both old-style
NLAs (where an NLA that targets a port/component ends in a
hw::InnerRefAttr) and new-style NLAs (where an NLA that targets a
port/component ends in a module/FlatSymbolRefAttr).
With these changes, InjectDUTHierarchy will reuse the existing DUT path
for components which use it inside the wrapper. A module target or port
target annotation on the DUT will use a clone of the original path (and
get a new symbol). This is a more efficient approach as it doesn't
require any walk of the DUT internals to find users.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Verilog attributes (SV 2017, section 5.12) support for regs and wires. Just like comments on modules, we're adding this support only where we need it since we'll have to reason about it in transformations.
This change updates the dedup pass to handle non-local annotations that
use a hierarchical path which ends at the parent module instead of the
annotation target. The main change needed is to track a list of
operations which reference the path operation, as multiple operations
can now share a single path. A follow up commit will change dedup to
produce the new style of path when it changes a local annotation into a
non-local one.
Remove internal NLA breadcrumbs that are unnecessarily generated in
EmitOMIR. These are entirely unused by internal EmitOMIR analyses and
serve no purpose other than to be removed by the pass later.
Change the EmitOMIR pass to remove no hierpath ops. It cannot safely do
this without computing the uses of a hierpath. While the pass could be
updated to do this, it would be cleaner to handle hierpath removal with
a common DCE pass.
This change is made to support new NLA-style paths. With this new
style, multiple NLAs may share the same hierpath ending at a module and
it is thereby unsafe for NLAs to decide to delete a hierpath without
checking for users.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Cleanup the EmitOMIR test by using {{LITERAL}} to avoid complex escaped
matches of verbatim symbol substitution, e.g., "{{0}}". Tighten up one
test that had already been migrated to using this by continuing to check
for the memory name (which needs to remain a regex match).
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Remove NLA breadcrumbs on instances from the EmitOMIR lit test. These
are unnecessary after fixes that have landed on master to remove
reliance on breadcrumbs.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Extend an existing test of EmitOMIR to also check new-style NLAs for
OMIR SRAM Paths. Specifically, check that EmitOMIR generates a symbol
for a symbol-less memory and that this generates the correct sv.verbatim
XMR path using this symbol.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Extend an existing test of GCT Views involving non-local annotations
that arise from deduplication to make sure that this works for the
new-style NLA format where hierarchical paths end in a module.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Some designs instantiate the same module more than once. Per-instance
data should be different, so we need a way to specify an "instance" of
the top level
This commit updates the ModuleInliner to not rely on circt.nonlocal
breadcrumbs and the NLATable analysis.
ModuleInliner uses a MutableNLA data structure to keep track of the updates
that are required on the HierPathOps without actually updating the HierPathOp
until the end of the pass. So, the algorithm has an invalid IR, while the pass
is running, and the IR is updated only at the end. This algorithm doesn't work
well with the NLATable, since the analysis expects the IR to be valid when the
NLATable is queried.
For example if we query the NLATable to get the HierPathOps that an InstanceOp
participates in, NLATable will lookup the namepath of a HierPathOp, but this
does not work if the HierPathOp is invalid!
So, this PR updates the ModuleInliner to not rely on NLATable and construct an
internal record of all the HierPathOps that each InstanceOp participates in,
and keep it up-to-date while the algorithm is running.
While the algorithm is running, an InstanceOp can be renamed, or the HierPathOp
that the InstanceOp participates in can be renamed. Hence, this PR adds the
DenseMap<InnerRefAttr, SmallVector<StringAttr>> instOpHierPaths;, which
records the HierPathOps that the InnerRefAttr corresponding to the InstanceOp
participates in. This essentially replaces the breadcrumbs on the InstanceOp.
Fixes#3307
Add `-mlir-disable-threading` to the LowerToHW diagnostic verification
tests since multi-threaded error reporting is non-deterministic due to
`mlir::failableParallelForEachN` not properly abandoning error reports
for iterations running in parallel with a failing one.
This commit fixes an issue with lowering unmasked RW memories.
If a memory has a single bit mask, then the mask bit can be removed and
combined with the write enable. But if the memory is a ReadWrite memory,
then the enable port is used for both read and write enable. For such a
ReadWrite memory we cannot combine the write mask with the enable, it has
to be combined with the Write Mode. This ensures that if the mask bit is
disabled, then the write should be disabled, but not the read.
This commit introduces a new lowering option to spill wires for expressions
with namehints. Previously, namehints are used only when they are
spilled to temporary wires. However sometimes we want to use verbose outputs,
so it would be good to have an option to probe expressions with namehints.
Unfortunately spyglass infers bitwidth of sub expression incorrectly
even when they are not toplevel. This commit will surpass the lint warnings.
This revert #3094 and adds explicit bitcast for every sub expression
when `explicitBitcast` option is true
This commit adds a pass `DropNamesPass` to change names to droppable
in order to disable name preservation. The pass walks `FNamableOp` and
drop their names. This pass should be executed in the very early pipeline
so that other passes can get more freedom about their names.
This PR adds NameKindAttr to explicitly represent name preservations of node, wire and register ops. Previously, `name` is directly used to represent the preservation kind (useless or interesting name) but it would be nicer to have the information explicitly, considering that name attributes are used as SSA names too. NameKindEnum has two elements, `InterestingName` and `DroppableName`. `InterestingName` must be preserved.
Example:
```scala
circuit Foo:
module Foo:
input a: UInt<1>
node b = a
node _c = a
```
Parse only output:
```mlir
firrtl.module @Foo(in %a: !firrtl.uint<1>) {
%b = firrtl.node %a : !firrtl.uint<1>
// same as %b = firrtl.node interesting_name %a: !firrtl.uint<1>
%_c = firrtl.node droppable_name %a : !firrtl.uint<1>
}
```
This just adds another test to make sure that we are recursively merging
operations with regions. I'm adding this test to make sure that this
doesn't regress from future refactorings.
Currently sv.namehint is not propagate well at comb canonicalizer. To improve the situation,
this commit replaces uses of `PatternRewriter::replaceOp` with its wrapper
`replaceOpAndCopyName` to propagate sv.namehint attribute more aggressively.
The assertion parsing framework would parse the label from
`cover:my-label:` as the message. This was a mistake in part of the
logic to handle missing labels in such cases as `cover:my message`,
where it would always use label as a message if the message was empty.
Now, it uses the colon to disambiguate the two cases.
As a follow up we should check if it is actually legal to have a missing
label.
When a wire has a single write and the write dominates all reads, we can convert the wire to a node. Wires and nodes are identically treated, except the connect indirection, so we can convert to a more convenient form.
This commit fixes an issue with lowering unmasked RW memories.
If a memory has a single bit mask, then the mask bit can be removed and
combined with the write enable. But if the memory is a ReadWrite memory,
then the enable port is used for both read and write enable. For such a
ReadWrite memory we cannot combine the write mask with the enable, it has
to be combined with the Write Mode. This ensures that if the mask bit is
disabled, then the write should be disabled, but not the read.
* Relax checks about ports driven multiple times or instantiate multiple
times, now that we support emitting references that
differentiate between them.
* NLA's are now allowed (used for differentiating if dedup'd, fancy
target paths, etc.), and converted into target refs. Target refs
start from NLA top unless there is a DUT on the namepath, in which case
that is the top.
* Symbols/inner_sym's used for as much as possible instead of strings
* In verbatim, track symbols to re-use indices
Refactor NLA and symbol-related tests in lower-types.mlir to live in
separate circuits. This is done in preparation for making updates to
code which changes these tests.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Remove tests in lower-types.mlir related to memory flattening. This
behavior was moved into the LowerMemory pass in ac5036c3b. ac5036c3b
also turned off these tests, but didn't rip out the tests. This commit
culls the dead tests/FileCheck lines.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Cleanup the lower-types.mlir test to remove long lines, put annotations
on separate lines, etc.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This improves the error message produced when there is an invalid flow
of a connect.
1. We stopped printing the MLIR connect operation itself. Even though
this is a verifier error, this is a user-facing error and we should
not be dumping it.
2. Start printing the name of declaration involved in the error message,
e.g. destination "x" has source flow.
3. Look through subfield operations when looking for the declaration
location and while printing the declaration name "x.x", which
provides accurate note declarations.
When the declaration referenced in a connect is an expression such as
`x + x`, we just skip the declaration name.
Change Grand Central (GCT) View behavior to always create an output YAML
file, even if there are no GCT Views in the circuit.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Adds support both at the front- and back-end level to ensure that all signed `arith` division operators work end-to-end.
Notably, this also changes the semantics of the `DivLibPipe` operator in that it no longer provides both a quotient and remainder through division. This change is motivated by the fact that:
- `arith.div` operations only provide quotients
- `comb.div` operations only provide quotients
As such, this change aligns the calyx division operators to better fit within an MLIR/CIRCT up/downstream context.
The loop bound computation previously relied on methods that attempted
to retrieve constant bounds, which would assert if the bounds were
non-constant (i.e. AffineMaps). This switches to use the same upstream
helpers that are used when lowering Affine loops to SCF loops, which
generate constant or dynamic Values using the Arithmetic dialect.
This fixes https://github.com/llvm/circt/issues/3127.
The MSFT dialect will be specializing modules based on a _per-instance_ basis. In practice, we expect this to be rather sparse. Rather than elaborating the entire design, we've chosen to select implementations based on the instance hierarchy path. So we need a way to know said path. Tracking it through module parameters fills that need.
As a first step, this PR adds a pervasive `__INST_HIER` module parameter during lowering and appends the instance name to it in every module instance.
We recently removed usages of the deprecated function `sextOrSelf`.
This function allowed you to extend an APInt to a smaller width than it
currently is, becoming a noop. The replacement function does not allow
doing this, and asserts. During the cleanup I missed that we had to
guard this usage of `sext` to check the initial bit width before
attempting to sign-extend.
See: https://github.com/llvm/circt/pull/3202
This commit supports FIRRTL_BEFORE_INITIAL/FIRRTL_AFTER_INITIAL
to inject statements before and after random initialization.
To maintain the relative order of ifdef blocks and initialization, sv.ordered is used.
This also fixes an invalid use of SmallDenseMap::[] which caused very weird
behavior.
Handle Cover operations differently than other verification ops, its expression/property works the other way.
Update canonicalization to elide cover(false) like we do assert(true).
Since graph regions don't have explicit output order, we add a region which outputs statements in order but is still a graph region. This let's macros which might attach to subsequent operations on expansion or other side-effecting code to be properly sequenced.
This commit adds a lowering-option "printDebugInfo" to emit debug information
directly to output verilog. This emits symbols on ports, wires and registers in the
comments.
This fixes an issue that version string is emitted to non-verilog files
such as json, metadata files #3169. This commit adds an additional flag
`isVerilog` to `FileInfo` so that we can emit version strings
only to verilog files in SharedEmitterState::collectOpsForFile.
Currently, files are regarded as verilog if output file names has ".sv" suffix.
Instead of directly referring to module ports, this commit changes the
GCT data taps pass to create an auxiliary "tap wire" inside the module.
The wire is connected such that it "observes" the current value of the
port (input or output), and all symbol references generated by GCT are
with respsect to this wire.
This has two distinct advantages:
1. It removes a major source of symbols on module ports, which block
port optimizations almost entirely.
2. It offers a "receptacle" for IMConstProp to propagate constants
into. This can lead to the tap wire being disconnected from the port
if the port is only driven to a constant externally, which allows
for the port to be removed.
Change GCT Data Taps to use a tap wire _for all_ taps, not just ports.
This is done to not block optimizations on tapped wires/nodes/registers
and to provide a constant receptacle for a constant value driven to
these wires.
Change Grand Central (GCT) Data Taps to sink trivial constants produced
by IMCP or canonicalization of "dead wire" taps into the actual data
tap. The symbol on the "dead wire" tap is then removed to enable it to
be removed.
This causes some problems with existing tests that use invalid or
constant values. Tests are updated to prevent this via either
DontTouchAnnotations or verbatim expressions (which block
optimizations).
Co-authored-by: Fabian Schuiki <fabian@schuiki.ch>
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
When emitting a module parameter which specifies neither a type nor a
default attribute, don't emit a type. Subsumes a string default since
the type of a StringAttr is NoneType.
This adds "// VCS coverage exclude_file" comment to modules under testbench to
make coverage accurate. The comment is added to extracted modules at ETC and
modules under testbench (ref #3058).
A logical, feed-forward connection between a producer and consumer. Can be
pipelined with a number of stages (cycle delay) on a per-instance basis.
`defaultStages` is used when `stages` isn't specified by a
`DynamicInstance`. Non-resettable, for now.
Per-instance specification is not yet supported, so the default pipelining
is always used.
This adds the `firrtl-` prefix to the beginning of the command line
option for this pass, so that we keep our dialect passes namespaced.
I found the use of capital letters in the command line option to be a
unconventional, so I spread it out to the significantly longer
`firrtl-mem-to-reg-of-vec`. In the future maybe we can think about
renaming this pass.
This reverts commit 76e38b19f6 due to this
tripping nightly integration tests failures with clang release builds.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit removes the NLA breadcrumbs from the InstanceOps.
This is part of the changes to port the passes to use NLATable analysis
instead of depending on the circt.nonlocal annotations on the InstanceOps.
Also, remove the code to update the NLA breadcrumbs.
And, remove the breadcrumb annotations from the lit tests.
This commit updates the ExtractInstances pass to use the NLATable analysis.
It also removes the updates to circt.nonlocal breadcrumbs on the InstanceOp.
Also update the lit tests to remove the breadcrumbs.
Modify Grand Central (GCT) Views to use read-only wire taps instead of
directly tapping the targeted signals. This change enables GCT Views to
not block optimizations due to tap points no longer getting inner
symbols (DontTouchAnnotations).
Improve Grand Central (CGCT) views handling to use constants instead of
XMRs when the XMR is tapping a wire/node that is driven by a constant.
E.g., GCT will now generate an XMR like the following where previously
it would XMR to a wire driven by a constant:
assign Foo.bar.baz = 1'h1;
When this logic is used, also remove the symbol on the tapped wire. This
effectively causes the constant tap to be a candidate for later DCE and
removes a source of dead wires in the output Verilog.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Printing the namepath array as a regular attribute means we have a lot
of overhead to write out all the `hw.innerRefAttr`s. This changes the
NLA operation printer to print the `namepath` array without the name of
the of the attributes.
Before: `firrtl.nla @nla_1 [#hw.innerNameRef<@Foo::@bar>, #hw.innerNameRef<@Bar::@b>]`
After: `firrtl.nla @nla_1 [@Foo::@bar, @Bar::@b]`
The `BlackBoxMemory` pass was an earlier implementation of SFC's
`ReplaceMemMacros` and has since been totally superseded by the newer
`LowerMemory` pass.
Move all logic related to Grand Central Data and Memory Tap annotation
scattering from `FIRAnnotations.cpp` into the `LowerAnnotations` pass.
Besides moving the scattering into `LowerAnnotations`, there are a few
additional changes to the actual annotations that are being generated.
Data and memory tap annotations identify an extmodule which is to be
synthesized into a data/mem tap module by the GCT pass. To capture which
synthesized port should tap which signal in the design, annotations are
generated in pairs: one on the extmodule port and one on the thing being
tapped. These used to share the same class name and were only
distinguishable by a `type = "portName"` and `type = "source"` field.
This commit removes this `type` field and generates annotations with the
distinct classes in the following cases:
- `ReferenceDataTapKey` becomes `ReferenceDataTapKey.port` and
`ReferenceDataTapKey.source`, respectively.
- `DataTapModuleSignalKey` becomes `DataTapModuleSignalKey.port` and
`DataTapModuleSignalKey.source`, respectively.
- `MemTapAnnotation` becomes `MemTapAnnotation.port`, `MemTapAnnotation.source`,
and `MemTapAnnotation.blackbox` respectively.
- `DataTapsAnnotation.blackbox` is added.
Furthermore, the `DataTapsAnnotation` that used to be placed on the data
tap extmodule is now called `DataTapsAnnotation.blackbox`. This change
is necessary to since `LowerAnnotations` has to distinguish between the
`DataTapsAnnotation` that requires custom scattering and the subsequent
simplified `DataTapsAnnotation` generated by that scattering which is
simply to be placed on the data tap extmodule.
Integration tests required no modifications, which is reassuring since
this commit attempts to only modify the intermediate representation of
GCT annotations before they are handled in the GCT data taps pass. This
GCT data taps pass tests were updated to match the new annotations. The
annotation scattering tests were migrated from `annotations.fir` into
`annotations.mlir` to match the move from `FIRAnnotations.cpp` into
`LowerAnnotations`.
This commit also adds a test for GCT mem taps on an actual `firrtl.mem`
op again which seems to have gotten lost sometime in the past and caused
a failure in an integration test that wasn't caught in the GCT data taps
pass tests.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Signed-off-by: Fabian Schuiki <fabian@schuiki.ch>
Co-authored-by: Fabian Schuiki <fabian@schuiki.ch>
This commit completes support for sign extension in SCFToCalyx, Calyx, and CalyxToHW. A new primitive, `std_extsi` is introduced to perform sign extension whereas `std_pad` implements zero extension.
Due some hasty changes when we realized that `MemToRegOfVec` transform
should not lower all memories, we started blackboxing seq mems which
we shouldn't be. We need to make sure that the `LowerMemory` pass is
gated on the `repl-seq-mem` flag in firtool, which will allow SRAMs to
be lowered to generated memory modules instead of blackboxes.
* [FIRRTL] basic support for subcircuit signal driving flows
This is handled with a two-step flow:
* main.fir + subCircuit.json --(firtool)--> (normal output) + sigdrive.json (new)
* subcircuit.fir + sigdrive.json --(firtool)--> (normal output)
The new test in 'subcircuit-flow' demonstrates this in action.
There are many limitations, this is meant to just be enough to
support some specific existing use cases only.
See PR for more details.
Change firtool to always run LowerAnnotations. Remove the "--new-anno"
option to conditionally run LowerAnnotations.
Move all logic related to Grand Central (GCT) view annotation scattering
from FIRAnnotations into the LowerAnnotations pass. In this commit
there are difficult to spot changes to parseAugmentedType and
applyGCTView functions (where the latter is extracted from the body of a
method inside FIRAnnotations):
1. Functions now use ApplyState instead of parameters passed to the
functions.
2. Any scattered annotations are handled by using ApplyState's ability
to add annotations to the worklist. E.g., parseAugmentedType will add a
new annotation to the worklist for every ground type leaf.
Co-authored-by: Will Dietz <will.dietz@sifive.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This allows a parametric module to instantiate nested parametric modules using
its own parameters.
This change required reworking the HWSpecialize pass, where parametric modules
defer specialization of instantiated parametric modules until after the parent
modules themselves are specialized.
Fixups to generate more `strictconnects` where we already intended to, and tree-wide cleanup to simplify and remove redundant helpers methods in favor of `emitConnect`.
Automatically inserting a pipe before the subpath makes it impossible to
correctly use `[0]` as the subpath, which is necessary for locating bits
in a register.
Improve and test the error message generated when a non-local annotation
is seen that LowerAnnotations knows is supposed to only be local.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This change specifies 2-space indentation to the JSON output stream
constructor, which causes the emitted JSON to be pretty printed. We
don't care too much about minimizing these files, and it was a pain for
humans to read.
`PDRegPhysLocationOp` takes a type and a list of physical locations. The
list of physical locations must equal the bit width of the type. The
physical locations are each optional.
Changes:
- Add `return failure()` where necessary now that `ParseResult` has
gained the `LLVM_NODISCARD` attribute.
- Adapt to new MLIR type alias syntax (no longer needs "type"), e.g.
`!foo = !hw.struct ...` instead of `!foo = type !hw.struct ...`
- Adapt to new attribute self-type parsing. Attributes now derive the
self-type from syntax like `#attr : !hw.some_type`. No longer need
an explicit type param `<T>` in stuff like `#firrtl.invalidvalue<T>`.
- Improve `add_circt_doc` to allow for multiple tablegen parameters and
add `add_circt_dialect_doc` to unify the way dialect documentation is
generated. The latter also passes the `-dialect` option to tablegen to
sort out issues in the generated documentation if a dialect
transitively includes another dialect. (This used to show up as
multiple doc files having the name of another dialect.)
- Add `let useDefaultTypePrinterParser = 1` and
`let useDefaultAttributePrinterParser = 1` to the dialect definitions
where appropriate, since these have become off-by-default upstream.
This commit sets the Combinational memory wrapper module name and file location,
instead of the generic "FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_a" formatted name and
default file location.
Use the name associated with firrtl.mem to derive the wrapper module name, after
appending it with _combMem.
This ensures the correct module prefix is used for the wrapper module. (For
example SiFive prefix for DUT modules and not otherwise).
For memories outside the DUT, set the output file directory to the TestBench
directory if available.
This change requires to add the MemOp to the FirMemory, such that the memory
dedup can be done in LowerToHW.
Use a map memoryNameMap to get the correct HWModuleGeneratedOp names for deduped
memories.
Blameless revert of e1cf64ad00. This is
done because there are some issues with the approach around
canonicalization that can cause intermittent failures in tests. This
will be fixed up in #3131.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
* [FIRRTL] Remove all-or-nothing LowerAnnos
Change the behavior of the LowerAnnos pass to work with existing
annotation scattering. Previously, use of the `--new-anno` firtool
flag (and backing FIRParser option) would cause ONLY the LowerAnnos pass
to be used. This changes that to use normal scattering and then apply
LowerAnnos. This is done to enable piecemeal migration of scattering
work into LowerAnnos.
* [FIRRTL] Make LowerAnnos search non-standard attr
Change the LowerAnnotations pass to only process annotations from a
non-standard attribute on the circuit with key
"annotationsForLowerAnnotations". This is done to enable the
LowerAnnotations pass to be run _after_ normal annotation processing
inside FIRAnnotations and FIRParser. This then enables us to migrate
annotations one-by-one from FIRAnnotation/FIRParser handling (by moving
the annotation into "annotationsForLowerAnnotations"). This commit does
not put any annotations into this area for processing. That will be
handled by subsequent commits.
* [FIRRTL] Add LowerAnnos passthrough in Parser
Modify the FIRRTL parser to let annotations that come in with the key
"annotationsForLowerAnnotations" to be placed in an attribute on the
circuit with this key. This is currently unused infrastructure that
lets annotations inside FIRAnnotations be put behind this key to let
them be handled by the LowerAnnos pass.
* [FIRRTL] Add isAnnoClassLowered util, NFC
Add a utility, isAnnoClassLowered, that returns true if an annotation
class name is known to be handled by the LowerAnnotations pass. This
effectively leaks internal information from the LowerAnnotations-defined
table of known annotations to let parsing/scattering logic know that
these annotations will be handled later.
* [FIRRTL] Skip LowerAnnos Annos During Parsing
Modify the Annotation parser to NOT handle annotations that are known to
LowerAnnotations. This is done to enable seamless migration from
annotation handling in the parser to annotation handling in
LowerAnnotations.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This adds initial support for converting Calyx to the "core" CIRCT
dialects: HW, Comb, Seq, and SV. This is done in a fairly direct way
using the dialect conversion framework. This is direct in the sense
that a Calyx reg is mapped to a Seq reg, a Calyx assign is mapped to a
SV assign, a Calyx std_add is mapped to a Comb add, etc.
I tried to port big chunks of the Calyx primitives over at once, when
the conversion could be templated and shared. There are still some ops
missing conversions.
I also deviate slightly in the behavior of pipelined ops, like
multiplication. This may lead to slight cycle-by-cycle differences
with the Calyx standard library implementations, but should still
produce functionally identical behavior.
The implementation makes use of lots of SV wires and assigns, in order
to stay simple and in-line with the form of the Calyx IR. These are
massively cleaned up by canonicalization, which can be run after the
conversion.
This commit introduces lowerings of the operations working on tuples.
TupleTypes are transformed to BundleTypes.
The pack and unpack operations connect the separated values with the
ones encapsulated in the bundle while having similar control logic as
ForkOp and JoinOp.
The metadata for seq_mems.json needs to be created before extract
instances runs, since it needs the original paths to the memory
operations.
This PR reverts the recent improvement to metadata emission, which used
symbols to ensure that the metadata reflected changes in the hierarchy.
It also modified the CreateSiFiveMetadata to pass to be able to run
before BlackBoxMemory. This just involved adding one extra annotation
to the list of known BlackBoxReader annotations.
Eventually, we will be getting rid of this particular piece of metadata, and
replacing it with OM.
This adds another heuristic to shouldSpillWire, this time to check if
the number of operands in a single expression inside a concat. If this
exceeds a configurable limit, a wire is spilled. This is to work
around Verilator #3405.
Large expressions inside a concat is worst case O(n^2) in a certain
Verilator optimization, and can effectively hang Verilator on large
designs. Verilator 4.224+ works around this by having a hard limit on
its recursion. Here we break large expressions inside concats
according to a configurable limit to work around the same issue.
See https://github.com/verilator/verilator/issues/3405.
This commit makes sure that the MemToRegOfVec transformation must run only if
the annotation "sifive.enterprise.firrtl.ConvertMemToRegOfVecAnnotation$" is
present on the Circuit op.
Earlier, the assumption was that the transformation can always run by default,
but in order to match SFC behavior this check is necessary.
Add the check and update all the lit tests accordingly.
Sequential memory read ports have enable inference when the address is
something with a name. We missed that using the subfield of an
aggregate does not count as something with a name, and should tie the
enable of the port to 1.
This adds a more complicated check to turn on enable inference for ports
which verifies that the value used is a port, wire, reg, regreset, or
node.
Previously, cells were instantiated with `FlatSymbolRefAttr`s but didn't actually use these in a symbol table context.
This commit changes the `CellOpInterface` to expect implementing operations to be symbol-defining operations. A side-effect of this is that we now implicitly check for duplicate instance names (something which was possible beforehand).
Add tests to `test/Dialect/FIRRTL/SFCTests` that specifically check if
the `ExtractSeqMems` portion of the `ExtractInstances` pass composes as
expected with other passes and features such as dedup, OMIR emission,
DUT hierarchy injection, and module prefixing.
Handle MemTap annotation on Memories that are excluded from `MemToRegOfVec`
transformation.
This fixes an issue with the commit #26276bf which updated this pass to only
work on registers that were generated by `MemToRegOfVec` transformation. The
assumption here was that every MemOp, should either be lowered to registers
by `MemToRegOfVec` or to `FMemModuleOp` by `LowerMemory`. So, the GCT pass
shouldn't expect to see a `MemOp`.
But yesterday's commit #b923963, updated the `MemToRegOfVec` to exclude
testbench memories but made the incorrect assumption that testbench memories
wouldn't have memtap annotations.
Finally this commit fixes that assumption and handles the Memtap annotation on
`MemOp`s.
This commit changes to lower node ops with symbols into read_inout ops of wires.
Considering the concept of the current name preservation, "nodes become wires
if they have names", it would be nice to use existing named wires as much as possible.
This change basically disables optimizations across named nodes at comb levels,
so we can prevent canonicalizers from duplicating expressions.
This change reduces 30% temporary wires and 50% lint warnings related to dead wires
in the large design while improving the simulation performance as well.
Fix an issue in the `ExtractInstances` pass where certain diamond-shaped
instantiation arrangements can lead to instances with identical symbol
after extraction.
With this change, the pass uses `ModuleNamespace::newName` to ensure the
instance symbol is unique within the module it is being extracted to.
Fixing this also requires fixing an issue where a black box in a
multiply instantiated parent would not properly filter its non-local
annotations to only contain the ones that go through the parent instance
it was pulled out of.
Fixes#3089.
Run the MemToRegofVec transformation only on DUT memories and ignore any
modules in the testbench. Also retain MemOps marked with
`ExcludeMemFromMemToRegOfVec` annotation.
LowerMemory pass should not error out on CombMems, but just retain them to be
lowered by HWMemSimImpl to registers.
The assumption here is that any operation which is not a special group operation (`calyx.group_done,calyx.group_go,calyx.assign`) is some form of combinational computation that can be lifted to the wire scope.
Extend the `ExtractInstances` pass such that it marks instances of the
new `FMemModuleOp` as to be extracted. This then allows the pass to
cover the functionality of the original `ExtractSeqMems` pass on the
Scala side of things.
Fixes#3092.
This changes the memory-related metadata emitted by this pass to be
created from FMemModules instead of MemOps. The memories will have
already been lowered into modules. The pass can be simplified because
it no longer has to worry about deduplicating the memories itself.
This changing the memory op lowering to store all annotations that were
attached to the MemOp on the new instance op, instead of on the new
memory module op. This is needed because OMIR trackers distinguish
their type based on the operation they are attached to.
This commit changes to reuse existing wires as placeholder of output ports
when output ports are uniquely assigned to a wire. This change reduces
the number of temporary wires in the large design 15%.
This commit adds extra parentheses around reduction ops (e.g. `&a` and `|b`) when
their operators are emitted in a row. For example, `foo & &a` becomes `foo & (&a)`.
Fix an issue in `InferResets` where the FIRRTL input would under certain
circumstances contain a zero-length vector of resets like `Reset[0]` but
without any accompanying `x is invalid` statement. This would be the
case for an optional list of resets where the Chisel code would connect
the individual reset wires, but would just emit no connects at all since
none would be present in an empty vector.
The original Scala implementation of `InferResets` seems to either not
have to deal with this issue since zero-length vectors are removed early
on, or it just ignores them for later Verilog emission to discard them.
This commit adjusts the behaviour of `InferResets` to always infer
zero-length reset vectors to `UInt<1>`, similar to how `InvalidValueOp`
is treated.
We should identify the clocks to be same even if the clock is passed via different wires.
This should prevent creating multiple write ports for combmem. This commit changes to
use `getModuleScopedDriver` to track clock port values.
Initial implementation of LLVM graph traits for an FSM machine operation. This will most likely be a useful piece of code for building further transformations, visualizations, ... upon.
When the SFC lowers memories to ext modules, they also create a wrapper
module which just instantiates the external module. MFC did not create
this wrapper module, since the extra level of abstraction did not seem
necessary. We have found that to be a drop in replacement for SFC, we
need these wrapper modules so that Module and Instance paths are all
identical. This commit adds wrapper modules following the style of the
SFC.
This commit also fixes how non-local annotations are handled by this
pass. When the memory operation has a non-local annotation attached, we
need to move the annotation over to the new module, update the NLA to
point to the module instead of the instance, and put an NLA reference on
the instance.
ExportVerilog may end up spilling temporary declarations for
expressions that are already going to be assigned to an inout. Try to
anticipate this in PrepareForEmission, and rewrite the uses of the
expression to read from the inout when possible.
This commit adds the MemToRegOfVec transformation to the FIRRTL pipeline.
This transformation converts all comb memories, to registers.
The mem tap and Async reset transformation are also updated to align
with this transformation.
(More details in the updated FIRRTL doc)
This transformation is enabled by default.
This adds the ability to lower FIRRTL memory modules to external module
black boxes. This lowering is almost the same as FIRRTL external
modules, but without having to worry about parameterization or defname.
This adds the ability to prefix and potentially duplicate the FIRRTL
memory modules. When the new memory lowering pipeline takes over, we
can remove the `mem.group` attribute which `PrefixModules` is using to
prevent later deduplication of memories.
This adds a new FIRRTL transformation which looks for
`AddSeqMemPortsAnnotation` and attaches the extra port specified to all
SRAMs under the DUT. The extra ports are always of type UInt<> with a
user specified width. The extra ports are wired up through the top of
the DUT and tied off to 0 in the testharness.
This pass requires that memories have been lowered to modules, and will
emit an error if it finds a regular mem op in the module. This is
because it is not possible to add arbitrary ports to a `mem` op that don't
match the schema of a memory `r`/`w`/`rw` port.
* LLVM bump (as we know it)
* minor format fix
* DCMAKE_BUILD_TYPE=?
* Update lib/Dialect/MSFT/MSFTOps.cpp
Co-authored-by: Andrew Young <youngar17@gmail.com>
* Update lib/Dialect/FIRRTL/FIRRTLOps.cpp
Co-authored-by: Andrew Young <youngar17@gmail.com>
* Update lib/Dialect/Calyx/CalyxOps.cpp
Co-authored-by: Andrew Young <youngar17@gmail.com>
* removed anon module test
Co-authored-by: Andrew Young <youngar17@gmail.com>
When the FullAsyncResetTransform replaces the reset of a register it
replaces all other connections to the register with a `mux(reset,
resetValue, originalValue)`. This caused problems when the value was
connected inside of a When block. The old reset value would become gated
on the When op's condition. The original reset value needed to have
higher priority than the original conditions. This change adds an
explicit self-connect to the register which is properly muxed with the
reset.
Input:
```firrtl
reg r : UInt<8>, clock with :
(reset => (reset, UInt<8>(1)))
when cond:
r <= x
```
Original output:
```firrtl
reg r : UInt<8>, clock with :
(reset => (async, UInt<8>(0)))
when cond:
r = mux (reset, UInt<8>(1), x)
```
New output:
```firrtl
reg r : UInt<8>, clock with :
(reset => (async, UInt<8>(0)))
r <= mux(reset, UInt<8>(1), r)
when cond:
r = mux (reset, UInt<8>(1), x)
```
The infer widths pass made the assumption that the arguments to a
`MuxPrimOp` were ground types, and would trigger an assertion trying to
create a `max(high, low)` constraint on the operands. This changes the
mux constraint generation to recursively constrain the type of the
result to the maximum of the high and low signals.
This commit introduces tuple types to the handshake runner. Tuple values
are represented as vectors of values.
Tuples can be feed to the handshake runner by using the `"(val1, ...,
valN)"` syntax.
This works similarly to output-omir: if specified, the final MLIR is
written into this file. This is useful to get the final MLIR output
alongside some other output, especially split-verilog.
This would close https://github.com/llvm/circt/issues/2946.
This adds a FMemModule operation, which is an external module used for
FIRRTL SRAMs. It is almost exactly the same, except it doesn't support
FExtModule's parameterization, does not have a `defname`, and it
contains required attributes for memory metadata. It was convenient to
not use FExtModules with a bunch of extra attributes stored on it to
make sure that the attributes were properly checked.
The set of attributes stored on the memory are those needed to create all
the metadata files. The types used for the attributes were generally
the same as those used by the memory generator schema. It might make
sense to revisit the types and use signless integer types in the future.
Co-authored-by: Prithayan Barua <prithayan@gmail.com>
This does a simple wire preservation. This means:
nodes and wires become wires if they have names
names that don't start with '_' are preserved
names don't block constant propgation, but do block deleting wires.
We don't have to extract assertions from modules in testbench since
it already separated from DUT. This commit addsfirrtl.extract.do_not_extract
attribue to modules on LowerToHW if they are located in testbench.
And in SVExtractTestCode, we skip extractions if that attribute appears.
This change will hopefully improve simulation performance by reducing overheads
of module instantiations.
Factors out functions for building constants and registers in Calyx programs.
Note: For some reason, this change seems to partially apply canonicalization in the `compile-control.mlir` test wherein constants are moved to the cell-scope.
The verification ensures that:
- At most one continuous assignment exists for any given value
- A continuously assigned wire has no assignments inside groups.
Add a `ExtractInstances` pass which implements a combined version of the
Scala `ExtractBlackBoxes`, `ExtractClockGates`, and `ExtractSeqMems`
passes. The pass is split into two phases:
As a first step it traverses modules and instances in the design and
marks the ones appropriately annotated for extraction.
In a second step it repeatedly pushes the marked instances up one level
of the hierarchy until they reach the desired parent module (usually
indicated with a `MarkDUTAnnotation`). The extracted instances are then
optionally grouped into a submodule at their extracted location. Finally
the pass also emits a text file indicating where the extracted instances
were originally located in the design.
Remove a check that asynchronous reset registers are only reset to
constant values from the LowerToHW conversion. This check has been
moved into the SFCCompat test.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Add SFC CheckResets logic to the SFCCompat pass. This checks that no
async reset initial value is a non-constant when looking through wires,
nodes, connects, and casts. An error and pass failure is generated if
this fails.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Change RemoveInvalid pass (yet again) to be called SFCCompat. This
better reflects that this pass is currently and will be later changed to
be where SFC compatible modifications or checks live.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit removes conditions of `isExpressionUnableToInline` regarding
users in different blocks. Dominance between user and def blocks is guaranteed
even in graph regions and because we are generating temporary wires at the
beginning of blocks, we don't have to check that condition today.
Since we currently only allow a single initial state for the state machine, it is a reasonable assumption that any unreasonable states may be pruned as part of canonicalization.
Previously, the first state in the machine was assumed to be the initial state - I believe being explicit here is important to ensure the machine behaves as intended. Also modified "default" state to "initial" state (seems to be more consistent with litterature).
When a module is not deduplicated due to a NoDedup annotation, the
equivalence checker would compare the module bodies, missing the fact
that the annotation was what blocked deduplication.
This also fixes a bug when the NoDedup and MustDedup annotation pointed
at the same module. We didn't record the presence of the module when it
was marked NoDedup which caused the MustDedup annotation to claim that
the referenced module did not exist.
* circt-reduce: cleanup --help output, put options in category
Hides unrelated options from LLVM+co, and makes it easier
to find the most relevant options at glance.
When building CIRCT against LLVM built as shared libraries
(as preferred by many Linux distributions) this greatly
reduces the number of options printed.
* firtool: cleanup --help output, put options in category
Hides unrelated options from LLVM+co, and makes it easier
to find the most relevant options at glance.
When building CIRCT against LLVM built as shared libraries
(as preferred by many Linux distributions) this greatly
reduces the number of options printed.
* llhd-sim: group llhd-sim options in --help output
Hides unrelated options from LLVM+co, and makes it easier
to find the most relevant options at glance.
When building CIRCT against LLVM built as shared libraries
(as preferred by many Linux distributions) this greatly
reduces the number of options printed.
Even for the statically linked version more commonly used
with CIRCT, the help output is significantly reduced.
* llhd-sim: touchup optimizationLevel cl::opt to be static (NFCI)
* test/llhd-sim: Add commandline test
* circt-reduce: check categories in help test
* firtool: check categories in help test
* firtool: add new option to mainCategory too
In the Scala FIRRTL Compiler (SFC), the instance paths to memories in
seq_mems.json and tb_seq_mems.json have a different root module
depending on whether the memory is located within the design-under-test
(DUT).
DUT memory paths should start from the DUT, while testbench memory paths
should start from the testbench (circuit top module).
Co-authored-by: Andrew Lenharth <darthscsi@users.noreply.github.com>
Co-authored-by: Andrew Young <youngar17@gmail.com>
* FIRRTLTypes: check for matching orientation on bundle elements
* FIRParser: don't use passive types when type-checking connect's
* FIRParser: fix debug taps creating newly invalid connect operations
The tap is set to be passive, so direct connect of bundles containing
flip orientations is no longer legal.
For these, break them and connect components.
* RationaleFIRRTL: fix field name in Bar flip example
Otherwise firrtl rejects this code regardless of type canonicalization
* connect-errors.fir: add test, examples from docs
* FIRParser: reuse expression, simplify a bit
As suggested in review, thanks!
* parse-name-preservation: check tap details
Don't only check that tap isn't created as it was previously.
Suggested in review, thanks!
* FIRRTLTypes.h: update comment for areTypesEquivalent
Sync w/comment on definition
* FIRParser: delete some dead code
The `HWInstanceOp` has a redundent check that the SymbolRef it has is a
valid HW module operation. This change keeps the more efficient of the
two methods, and remove the slower one.
Add a pass that adds a level of hierarchy (indicated by an
InjectDUTHierarchyAnnotation) above the DUT (as indicated by a
MarkDUTAnnotation).
Add an option to enable the InjectDUTHierarchy pass from a command-line
option in firtool.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
When the ReadInOutOp accessing a register or a wire was after the use of the read value, a temporary wire was emitted. This patch instead moves both the definition of the read operation and the read itself to the top of the module, ensuring the order is correct and no temporaries are needed.
As a result, the verilog representation of a large design is 0.6% shorter, with thousands of temporaries eliminated.
Create an instance hierarchy root operation. Require dynamic instances
to nest inside of it and only represent a single level of the hierarchy.
Results in a tree which better models the instance hierarchy.
This commit adds an extra initialization for async registers so
that they are set to initial values if reset signal is enabled at
an initial randomization phase. This matches SFC's behaviour.
Close#2999.
This commit tries improving CheckCombCycles's error message by recursively
inspecting instances and recovering a full cycle. Previously it didn't recurse into
instance's internal so the message is less informative than SFC.
* [FIRRTL][ModuleInliner] Add test for inlining flattened module multiple times
This test presently fails.
* [FIRRTL][ModuleInliner] Don't remove flatten annotation immediately
We need to keep it around to know to inline-flatten
should it be visited again (multiple sites, for example),
and to ensure flattening occurs when it is reached in the worklist.
This fixes TestInliningFlatten, but breaks annotation cleanup.
This partially reverts e1709ff509.
* [FIRRTL][ModuleInliner] Fix removing flatten annotations
The previous code removed it from the temporary AnnotationSet object,
not the module itself. Also the code skipped this line when the
annotation was present (shouldFlatten was true -> continue).
Now the annotation is removed (only) when the module has been flattened.
It may still be visited for inlining on a separate path,
but since flattening has occurred just inlining as normal is fine.
(inlineInto instead of flattenInto).
* [FIRRTL][ModuleInliner] Add test for annotation removal on public modules
This test currently fails, as InlineAnnotation lingers on public modules.
These aren't removed explicitly as previously all inlined modules
became dead and were removed (taking their inline annotation with them).
* [FIRRTL][ModuleInliner] Clean up lingering inline annotations
While walking modules anyway, assert no remaining (live) modules still
have the flatten annotation as these are removed when flattening.
Their presence at this point suggests a bug of some kind.
This change fixes TestPubAnno.
* [FIRRTL][ModuleInliner] Quiet tidy remarks about recursion (NFC)
Currently, when passed `--repl-seq-mem-file=mem.conf` and there are no
memories, we will not emit the `mem.conf` metadata. This changes that
behaviour to create an empty file, which is the same as SFC behaviour.
Some tooling expects this file to be created even if it is empty. We
made a similar change for `seq_mems.json` and previously decided we
didn't need to have the same behaviour for this file.
The `LowerMemory` pass currently just flatten aggregate datatypes into a
single uint. I want to separate the logic which lowers memories into
`hw.generatorModule`s out into a different pass, and I want to use the
name `LowerMemory` for that.
Create a new pass to flatten the aggregate memory instead of doing it in LowerTypes.
This commit removes the logic from LowerTypes and moves it into a new pass that
should run before LowerTypes.
Update the memory module name and instance names with an "_ext" suffix to
match SFC behavior. This commit adds a utility to Namespace, to generate
a unique new name with a given suffix.
This utility is used to generate the memory module name with "_ext" suffix.
This commit also updates the LowerToHW to add the "_ext" suffix to
the instance name.
When two modules marked with MustDedup are not deduped with each other,
but they are deduped with something else, we end up trying to compare
the body of a module which was deleted. This change makes us look up
the module they were actually deduped with to compare the bodies.
Make the GroupID memory attribute as unsigned int, instead of signed.
It is being used to distinguish memories that should not be Deduped.
There are two passes currently updating the GroupID:
PrefixModules pass assigns unique id for each prefix.
CreateSifiveMetadata pass assigns unique id for each memory under
the testharness hierarchy.
This change ensures consistent assumptions and a convention.
Cmdline flags to deal with subcircuit. subcircuits need to print xmrs relative to the DUT, which they don't know due to their separate compilation. The toplevel module might also have been prefixed. This is all less than ideal, but the mechanism is expected to be removed from Chisel soon, so this is a stopgap.
This change ensures the testharness memories are not deduped,
irrespective of the GroupID assigned to them.
It ignores the GroupID assigned to a testharness memory by PrefixModules
pass, which is currently the only pass that assigns the GroupID.
Fix a bug in Grand Central (GCT) Signal Mappings (SM) where a forced
input would unintentionally cause more than just the port to be forced.
SystemVerilog force semantics drive the entire net when a port is
forced. Because CIRCT does not generate a dedicated wire for each
instantiated port (like the Scala-based FIRRTL Compiler does), this can
cause the force to be more broadly applied than the Chisel user intends.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
In Grand Central (GCT) Signal Mappings (SM), erase all connects that
drive sources. The Chisel-level GCT API had been modified to emit
sources as invalidated wires to work around CIRCT running GCT passes
after ExpandWhens (which is when initialization checking happens).
Without removing these drivers, the final circuit winds up with multiple
drivers to the same net type which most simulators will interpret as
"x".
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Update the Grand Central (GCT) Signal Mappings pass to emit SiFive test
configuration JSON.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Add an attribute to the FIRRTL Circuit-level Grand Central (GCT) Signal
Mappings (SM) annotation to indicate if JSON should be emitted during
the GCT-SM pass. This effectively encodes if firtool is compiling a
subcircuit (JSON needs to be emitted) vs. firtool is compiling the main
circuit (JSON should not be emitted).
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit adds a new reduction named NodeSymbolRemover to
remove symbols on node operations. This reduction is intended to
clean up symbols created by name preservation.
This commit adds `LocationInfoStyle` enum to control location info
in output verilog. Specifically we want to distinguish location information
from normal comments more clearly. As one implemention, `WrapInAtSquareBracket`
option is added to emit location information wrapped in `@[` and `]`
such as:
```
assign .. ; // @[perf/regress/AddNot.fir:3:10, :7:{10,17}]
```
This fixes a bug introduced by 286c483. An iterator is mistakenly incremented in certain execution. Since opIterator is incremented in the beginning of the loop,
we are not actually processing next operations. This causes an assertion failure
on ExportVerilog in internal designs.
This commit adds a unique groupID to each Testharness memory to prevent their
Dedup. This is a temporary fix to match the SFC behavior.
For any memory under the Testharness hierarchy, assign a unique groupID to it.
This pass uses the max unsigned integer to generate the groupIDs, the assumption
being that, it will never conflict with an existing groupID. The groupID will ensure
that they are not deduped.
Note: Currently, the memory dedup happens during the metadata emission, because
that is the first pass that requires the name of the final memory module. The plan
is to create a new Memory lowering pass, and move all the memory transformations to it.
This aligns the textual IR format, the emitter, and any locations
these ops were constructed in code to the result order defined by the
native Calyx compiler. This is required for these ops to be able to
round-trip through the native compiler.
Change the Grand Central (GCT) Signal Mappings pass to be a pass that
operates on the whole FIRRTL circuit instead of on a module. Continue
to keep its parallel execution behavior. This is done in preparation
for modifications to the pass to support emission of a JSON structure
necessary for SiFive tooling which requires full-circuit information.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit adds `print-simple-cycle` option to print a simple cycle.
With that option, CheckCombCycle pass prints signal names contained
in a cycle (in the order of a cycle path) instead of dumping all
operations in the scc. It is useful for chisel users to debug their
chisel code since the cycle is actually a counterexample. Previous
error message (i.e. dumping all operations in SCC) is intentionally
kept to test the cycle dectection algorithm.
Fix Grand Central to properly handle views/interfaces that are
instantiated in the test harness and not in the DUT. Specifically,
interfaces and companions are not extracted via binds if they are only
instantiated in the testharness (and a DUT is known via a
MarkDUTAnnotation).
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
The previous verifier required sv.wire to exist in the top-level body
of a hw.module. This relaxes the verifier to allow wires in any
non-procedural region, which the NonProceduralOp trait already
guaranteed for sv.wire.
Fix a bug in Grand Central (GCT) signal mappings where a 0-width target
would crash compilation when either compiling the main circuit or the
subcircuit. Avoid the issue entirely by dropping targets with a zero
width inside the GCT signal mappings pass. If a module is exclusively
driving zero-width targets, then don't generate an empty mappings
module.
This is done to work around an issue with some SiFive designs where this
pattern is used.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
A memory operation has a name attribute which is used for both the name
of the generated module and all instances of that module. The
ModuleInliner pass is supposed to prefix the name of instance but not
the name of the module. This change stops prefixing the memory, which
corrects the name of the module, but gives the incorrect instance name.
The assumption here is that its more important to get the name of the
memory module correct than the instance.
Handle zero bit memory data fields correctly when flattening aggregate memory data.
This commit ignores the zero bit data field, and also the corresponding mask bit.
Fixes#2926
Fix a bug in the GrandCentralTaps pass where a data tap module that had
no ports would result in external module instantiations in the final
design that had no corresponding implementation. Change the
GrandCentralTaps pass to delete these modules and their instantiations.
This can arise more frequently due to a recent change to the Grand
Central Chisel API where zero-width taps are now allowed in the API,
generate a tap module, but generate no actual taps.
An alternative implementation would be to generate empty modules which
are cleaned up later. This seemed wasteful.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
This commit introduces the `-hw-specialize` pass. Any `hw.instance` operation instantiating a parametric `hw.module` will trigger a specialization procedure which resolves all parametric types and values within the module based on the set of provided parameters to the `hw.instance` operation. This specialized module is created as a new `hw.module` and the referring `hw.instance` operation is rewritten to instantiate the newly specialized module.
The intention of this is to have a path for:
1. Writing generic modules ->
2. Specialize module instances ->
3. module inlining ->
4. inline optimization
When the LHS and RHS of a partial connect were the same types, we would
just emit a regular connect. We forgot to check if there were any
analog types inside the bundle When this is the case, we have to recurse
on the bundle elements and attach the analog fields together.
Don't generate always blocks with ifdef in them for side effecting operations, generate ifdef with always in them. This reduces preprocessing work during synthesis and allows merging only verif op's always blocks.
This commit adds a new lowering option emitReplicatedOpsToHeader
to create a header file which stores replicated ops.
Currently, this PR creates circt_header.svh under the output
directory and users are expected to manually register the header to their
include paths.
Change Extract Test Code (ETC) behavior to not perform extraction if the
module is already going to be otherwise extracted via a bind. This
fixes an issue where certain simulators are unhappy if you bind into a
bound instance.
Fixes#2910.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Extend FIRRTL's RemoveUnusedPorts pass to also remove ports when a port
is never used by any instantiation. While seemingly uncommon, this can
happen frequently for certain Chisel utility modules which have
auxiliary ports which are very rarely used, e.g., for Queues or
Arbiters.
Fixes#2850.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
A user can add an annotation "MustDedup" which lists modules which they
expect to dedup, and emits an error message when that fails. The error
message produced by a dedup failure doesn't give any hint as to why
deduplication failed. This can be very annoying to debug, and usually
involves diffing the FIRRTL IR modules.
This change adds a diagnostic comparison of two modules, looking for the
first difference between them which would have blocked deduplication.
This comparison ignores differences in port names and bundle element
names the same way that the structural hasher does.
This approach is still not super useful when trying to debug
deduplication failures: it will print the first difference found in the
modules and then bails out. This can help with simple things like when a
wire has a different type and you can easily map it back to the original
Chisel code. For differences in complicated logic, it can be challenging
to map it back to the chisel code and even harder to figure out why
these were emitted differently.
Fix a bug in the verbatim Verilog created by HWMemSimImpl for
randomizing memories. This previously relied on an invalid Verilog
construct (bit select out of a concatenation). This is changed to be
legal by using a temporary register for memory initialization.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
* [Conversions][Transforms] Add missing includes due to refactoring in upstream MLIR.
LLVM commit 36d3efea15e6202edd64b05de38d8379e2baddb2 removed a few
include statements from mlir/Pass/Pass.h, so we have to update a few of
the files here to add back in some includes.
* [Handshake][SV] Migrate from StrEnumAttr to EnumAttr.
StrEnumAttr was removed in upstream LLVM commit
60e34f8dddb4a3ae5b82e8d55728c021126c4af8. It has been replaced with
EnumAttr, which can do everything that StrEnumAttr could do but more
efficiently.
This affected two specific attributes: Handshake's BufferType, and SV's
ModportDirectionAttr. Both have been converted over to EnumAttrs. Their
parsers had to be updated, since the new EnumAttr no longer likes having
double quotes around the enum value in the assembly format.
Consequently, the tests were also updated.
* Bump llvm to 1aa4f0bb6cc21b7666718f5534c88d03152ddfb1.
Added a separate flag to dump the IR with `sv` ops, replacing `--ir-hw`.
From this point onwards, `--ir-hw` will dump IR in HW form, while `--ir-sv`
will include constructs (such as registers) expanded into SytemVerilog form.
Add an attribute to memory, to prevent memories with different prefix from
getting deduped. The `PrefixModules` pass can prefix the name for a `MemOp`.
This prefix should not be ignored when memories with same properties are
deduped later (during Metadata emission or LowerToHW).
This commit adds the optional `groupID` attribute to the `MemOp`, such that
it should also be considered when merging all the memories.
This commit fixes the annotation used to specify the modules that need
Retiming, it was incorrect earlier.
Also rename the filename annotation variable to avoid confusion.
And update the doc and lit tests with the correct annotation.
* [FIRRTL][Dedup] Fix hash() to work with new SHA256.final() return type.
This follows the commit in upstream llvm,
330268ba346b679af786879d8f696c8c412a40eb, which changed SHA256.final()
to return a std::array<uint8_t, 32> instead of a StringRef. Although it
would be nice to directly use the SHA digest bytes as the hash key,
DenseMap does not have an implementation of DenseMapInfo for
std::array<uint8_t, N>, so it's easier to continue converting this to a
string for now.
* [LLHD][SCFToCalyx] Update getSuccessorOperands() to match upstream MLIR.
Commit 0c789db541c236abf47265331a2f2b0945aa7b93 in upstream llvm changed
BranchOpInterface's interfaces for getting successor operands.
It removed getMutableSuccessorOperands() and it changed
getSuccessorOperands() to return a new SuccessorOperands object instead
of an Optional<OperandRange>.
This affected LLHD's WaitOp, which implements the BranchOpInterface, and
SCFToCalyx, which calls some of the methods on BranchOpInterface.
For llhd::WaitOp, we now implement getSuccessorOperands() instead of
getMutableSuccessorOperands(), with the appropriate return type.
For SCFToCalyx, we now always work with a non-Optional SuccessorOperands
object, which required updating a few of the usages to not call Optional
methods.
* [Analysis][Scheduling] Explicitly set TypeID for test passes in anonymous namespaces.
This matches upstream LLVM commit
5e50dd048e3a20cde5da5d7a754dfee775ef35d6, where types defined in
anonymous namespaces are no longer allowed to have implicit TypeIDs.
This mostly only affects test passes.
* Bump llvm to 3d4ca8a8c39f772dd6c022220a6eef23238a77f6.
* [LLHD] Mark probeCSE test with XFAIL.
This test started failing after an LLVM submodule update. It is possibly
related to upstream LLVM commit
02da9643506dee4a82353e0f911513279634d846, which changes the behavior of
the upstream MLIR CSE pass to be able to optimize the case where there
are side effectful reads without an intermediate side effectful write.
This test case is now CSE'ing the `llhd.prb` ops into a single op and
returning both copies of it.
Extend the RemoveInvalid pass (formerly RemoveResets) to convert all
invalid values to zero. This is done to apply the "invalid is zero"
interpretation after all other invalid interpretations have been
resolved. This replaces logic where this interpretation was
incompletely applied during canonicalization after RemoveResets.
Fixes#2782.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Change the name of FIRRTL Dialect's RemoveResets pass to RemoveInvalid.
This is done in preparation to add functionality to RemoveResets to
remove all invalid values after it removes resets. In effect, this pass
is now an "invalid removal" pass.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Fabian Schuiki