Commit Graph

3356 Commits

Author SHA1 Message Date
Schuyler Eldridge bf6b18f494
[FIRRTL] Canonicalize Invalid Connect to Zero (#2288)
Extend the behavior of connect canonicalization to convert a connection
to an InvalidValueOp to a connect to zero.  This is done to enforce
compliance with one Scala FIRRTL Compiler (SFC) interpretation of
"invalid is constant zero".

This change requires a RemoveResets pass (#2287) to run before the
canonicalization that this PR adds.  This is because registers require a
separate interpretation of invalid where "invalid is undefined when used
as the initialization value of a register" (and where invalidness is
determined by looking through wires/connects in the current module).
This latter, "register initialization" interpretation needs to run
before invalids are fully removed by the code added in this commit.

Add an end-to-end test of firtool that checks for compliance with the
multiple Scala FIRRTL Compiler (SFC) interpretations of invalid.  This
test is added to lock in behavior around invalid as this is known to
cause formal equivalence failures, e.g., if a register has a reset in
CIRCT, but does not in the SFC formal equivalence will rightly complain.
This test can likely be removed or weakened once better semantics for
invalid value are defined other than "whatever the SFC does".

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-06 17:29:47 -05:00
Schuyler Eldridge e5f0c5065e
[FIRRTL] Parse params < 32-bit as 32-bit (#2300)
Change FIRRTL parsing to convert all parameters less than 32-bit to
32-bit APInts.  This fixes downstream issues with Verilog emission,
where negative parameters may have too few bits in their APInt
representation and print as shorter-than-expected literals.

Fixes #2299.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-06 17:03:11 -05:00
Schuyler Eldridge b31ce1e65a
[FIRRTL] Add RemoveResets Pass (#2287)
Add a new pass, RemoveResets, that replaces RegResetOps that have
invalidated initialization values with RegOps.  This is part of a series
of patches that are intended to align CIRCT with the Scala FIRRTL
Compiler (SFC) interpretation of invalid.  Previously, CIRCT relies on
canonicalization/folding of invalid values to do this optimization.
This pass enables future canonicalization/folding of invalid values to
zero (as the SFC does) without having to worry about performing this
optimization.

Run the RemoveResets pass as part of firtool after ExpandWhens and
before the first canonicalization.  This enables conversion of
invalidated RegResetOps to RegOps before canonicalization (eventually)
interprets invalid values as zero.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-06 16:13:27 -05:00
Schuyler Eldridge 635786e1cb
[FIRRTL] Rename a "fold" to "canonicalize", NFC
Rename the private "foldSingleSetConnect" function to
"canonicalizeSingleSetConnect" as this is actually only used by the
ConnectOp canonicalizer.  This name change is done purely for clarity as
multiple people have been confused thinking that this function was a
folder or used by other folders (when it is not).

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-06 15:32:13 -05:00
Prithayan Barua 6f9d5515a5
[SV] Add option to ignore read enable (#2271)
This change adds an option to ignore the memory read enable signal during
 memory lowering to register banks.

According to the FIRRTL spec, the read output is undefined if the read
 enable signal is disabled.
But the `firrtl` implementation ignores the read enable signal on latency 0.

This change ensures that, in `HWMemSimImpl` by default the read output is set
 to `X` if read enable is disabled, but if `ignore-read-enable-mem` option
 is passed, then the read output is set with the data at the read address,
 irrespective of the enable signal.

This provides an option to match the Scala FIRRTL compiler behavior.
2021-12-06 06:29:12 -08:00
Morten Borup Petersen b96581a7b7 [handshake-runner] Fix issue in arith_cast execution 2021-12-06 12:53:10 +01:00
Morten Borup Petersen 450015a448 [handshake-runner] Use ODS operands where applicable 2021-12-06 12:53:10 +01:00
Morten Borup Petersen cbec276abc [handshake-runner] Support multiple-input mux ops 2021-12-06 12:53:10 +01:00
Hideto Ueno a86ac4aa59
[FIRRTL/FIRRTLFolds] Fix a bug in getConstant (#2292)
This commit fixes a bug introduced by c351dc6017. `BoolAttr` is also an `IntegerAttr` so we need to check `BoolAttr` first.
2021-12-06 20:47:30 +09:00
Schuyler Eldridge b4eec0423e
[FIRRTL] Fix SubPrimOp canonicalization bug (#2290)
Fix a bug in SubPrimOp canonicalization where an asUInt cast was applied
before the pad inside a utility method.  This commit changes the utility
method to apply a cast (either asUInt or asSInt) if needed.

Fixes #2289.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-05 12:28:29 -05:00
Fabian Schuiki c351dc6017
[FIRRTL] Improve canonicalization involving `invalidvalue` (#2251)
This extends the FIRRTL fold and canonicalization patterns to treat
`invalidvalue` as a zero if it is an input to a primary operation. This
is inline with what the Scala implementation of the FIRRTL compiler
does.

Doing this removes quite a few of the special cases where certain ops
are aware of `invalidvalue`s and will fold as if it was a zero. The
addition of the `getConstant`, `isConstantZero`, and
`canonicalizePrimOp` helpers allow for most folders and canonicalizers
to not having to bother with `invalidvalue`s, as these helpers
implicitly map `invalidvalue`s to zero constants.

This also subsumes the work done in #2278 and #2198, which selectively
added handling for `invalidvalue`s to the `PadPrimOp`.

The extension of the folders and canonicalizers now makes all the cases
in `invalid-reg-fail.fir` pass, albeit some only after accepting that
the MLIR implementation folds *additional* cases where the Scala one
might just leave an operation untouched. These are marked with a
`<-- fixed; upstream to Scala FIRRTL impl?` and should probably be
upstreamed as additional canonicalizations on the Scala side as well.
2021-12-04 09:22:04 +01:00
John Demme 18868ce742
[ExportVerilog] Implement `sv.namehint` (#2281)
Use `sv.namehint` instead of just `name` in ExportVerilog. Update
rationale. Closes #1752.
2021-12-03 14:10:22 -08:00
mikeurbach fa29173eaa
[Moore] Specify -attrdefs-dialect when generating attributes, NFC. (#2280)
This flag appears to be needed to support an upcoming revision of
LLVM. Without this, TableGen complains with:

```
error: defs belonging to more than one dialect. Must select one via
'--(attr|type)defs-dialect'
```
2021-12-03 13:14:36 -07:00
Hideto Ueno d7f29f3d76
[FIRRTL/ExpandWhens] Support vector types (#2286)
This commit changes expandWhens to accept vector types under the condition that connections are expanded, i.e. all the connections are between ground types. This commit changes `declareSinks` to handle vector types.  Currently, aggregate registers are not supported and it will be supported after implementing expand connections.
2021-12-04 04:49:02 +09:00
Andrew Young eafba50792
[FIRRTL] Add method to insert ports into instance ops (#2275)
This adds a method to insert ports into an instance op.  Since this
increases the number of results of the instance op, it actually has to
be cloned.  It is assumed that the user will manually call
`replaceAllUses` and erase the original instance operation.  This
functionality is useful for any pass that adds ports to modules.
2021-12-03 09:21:54 -08:00
Andrew Young 9e74f40f82
[FIRRTL] Make Annotation hold on to the actual attribute (#2276)
When an annotation targets only part of the thing it is attached to, it
is represented using a `SubAnnotationAttrs`, which contains the field ID
of its target.

The Annotation wrapper class can be only be created from a
DictionaryAttr.  When using a SubAnnotationAttr, it is created from its
internal DictionaryAttr, and the stored field ID is ignored. This makes
it impractical to use `SubAnnotationAttrs` with most of the Annotation
APIs.

This change makes it so the Annotation wrapper holds on to the original
annotation attribute, instead of just a DictionaryAttr.  The annotation
wrapper will check which kind of annotation it has internally when calls
are made.

This change then adds a call to `getFieldID`, which return 0 for regular
annotations, and the field id for `SubAnnotations`.

This also changes the equality comparison for two annotations: a
Annotation created from a SubAnnotationAttr will no longer be equal to a
regular annotation when the dictionary attribute is the same.
2021-12-03 09:21:21 -08:00
Morten Borup Petersen f1b191d397
[Handshake] Add buffer removal pass (#2285)
A very simple pass which removes any buffers present in the circuit. This pass can come in handy if:
1. one would like to re-buffer a circuit
2. one would like to simulate without buffers
3. one would like to simplify the circuit (for visual/.dot inspection)
2021-12-03 17:28:59 +01:00
Fabian Schuiki 22d0ac6e62
[FIRRTL] Typos, comments, whitespace; NFC
Fix a few typos and comments in the canonicalization tests and impl.
2021-12-03 13:56:52 +01:00
Fabian Schuiki 45212fbc1d
[FIRRTL] Assert getExtendedConstant only used for int types 2021-12-03 13:50:55 +01:00
Fabian Schuiki 0c913f5d92
[FIRRTL] Fix gt/lt/geq/leq constant-to-RHS canonicalization
Fix an issue in the canonicalization patterns for the gt/lt/geq/leq
prim ops where a `SpecialConstantOp` (can never occur) or
`InvalidValueOp` (*can* occur) would not be properly rotated to the RHS.
2021-12-03 13:49:09 +01:00
Fabian Schuiki 133c055299
[FIRRTL] Cleanup some ambiguities about empty port anno/sym arrays (#2147)
In certain places, FIRRTL would accept empty port annotations and symbol
arrays as a shorthand for "none of the ports have any anno/sym", while
others do not. This cleans up hopefully all of these uses to
consistently accept empty arrays as shorthand.
2021-12-03 09:39:17 +01:00
John Demme 41432eb2d2 Fix the integrations tests broken by "[Comb] Remove the now-unused comb.sext operator." 2021-12-03 07:31:37 +00:00
Chris Lattner c986956a56 [Comb] Remove the now-unused comb.sext operator.
This is better modeled with existing replicate/extract/concat
primitives, which are more expressive and general.

This resolves Issue #2248
2021-12-02 22:34:28 -08:00
Chris Lattner 0fb933bc93 [tests] Move all tests off comb.sext. NFC. 2021-12-02 22:30:25 -08:00
Chris Lattner 344a73c9ec [CombFolds] Add a canonicalization for extractop that happened for sextop.
We looked through multiple operations using computeKnownBits in
SExtOp::canonicalize, which caught one fold in RocketCore.fir.  Add
that for ExtractOp.
2021-12-02 22:11:44 -08:00
Chris Lattner 59e57a87a5 [LowerToHW] Generate extract+replicate+concat instead of SExtOp.
The former is more canonical and has more transformations that work
with it, e.g. we now generate this in RocketCore.fir:

  {{65{remainder[7]}}, remainder[7:0]}

instead of:
  {{65{_remainder_7to0[7]}}, _remainder_7to0}

because we don't have the replicate/extract buried in the SExt.
This is part of Issue #2248.
2021-12-02 21:58:29 -08:00
Chris Lattner 1be7c22963 [CombFolds] Canonicalize repeated concat operands into replicate.
This provides a generalized canonical form for concatenations of
repeated elements.
2021-12-02 21:31:53 -08:00
Chris Lattner fc84e739ed [CombFolds] Canonicalize sext(onebit) -> comb.replicate.
This gets us building more replicate ops in practice, and allows
moving one pattern over.
2021-12-02 16:21:00 -08:00
Chris Lattner d535c5c549 [CombFolds] Canonicalize concat(a,a,a,a) -> replicate.
This allows removing a special case from ExportVerilog,
and gets us using replicate in practice.
2021-12-02 15:59:07 -08:00
Chris Lattner 9c39489510 [CombFolds] Canonicalize extract(replicate). 2021-12-02 15:54:48 -08:00
Chris Lattner e97bd6c496 [Comb] add a new ReplicateOp with ExportVerilog support.
This follows the design of the SV spec, and is a more principled
ingredient to implement sign extend support.  This is part of
Issue #2248.
2021-12-02 14:28:20 -08:00
Chris Lattner 2b9991b8e9 [CombFolds] Use convenience ExtractOp builder to simplify code, NFC. 2021-12-02 12:46:39 -08:00
Schuyler Eldridge 2a6f621b88
[FIRRTL] Fold pad(invalid, width) -> zero<width> (#2278)
Change the behavior of folding around pad, as discussed in #2197 and
implemented in #2198, to fold a pad of an invalid value to a constant
zero.  This prevents propagation of invalids which is generally viewed
as a source of errors.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-02 14:36:49 -05:00
Andrew Lenharth 2c5f5a81da [NFC] Use new memop builders 2021-12-02 12:04:17 -06:00
Morten Borup Petersen 33c4c645b8
[Transforms] Add --flatten-memref-calls pass (#2257)
This pass is intended to be a "software"-like companion to --flatten-memref. Whereas --flatten-memref will flatten all memref operations and function signatures to unidimensional memories and accesses thereto, this pass can be used to transform calls to such flattened operations. In other words, this pass expects the parent function of a `callOp` to be able to support multidimensional memories, but rewrites calls _to_ flattened functions.

This is done by converting memref arguments through a `memref.subview` operation. Any called function which had its signature rewritten will be replaced by a private function definition having a signature of the rewritten function type.
2021-12-02 10:49:07 +00:00
Morten Borup Petersen 9bc200c6ef
[StandardToHandshake] Actually fail conversion when raising errors (#2267) 2021-12-02 10:48:31 +00:00
John Demme 7cf4f5e365 [PyCDE] Hide support functions which convert objects from Python to MLIR
The `obj_to_value` and `var_to_attribute` functions used to be publicly
exposed since they had to occasionally used by users. Now that we've
patched all the cases, "hide" them. Also rename to be more consistent
with each other.
2021-12-02 05:22:57 +00:00
John Demme be39c7d18f [PyCDE] Change <type>.create(obj) to <type>(obj)
Make it feel more like a class instantiation.
2021-12-02 05:17:44 +00:00
Hideto Ueno 0269984510 [SV] Use assembly-format for interface and modport types, nfc 2021-12-02 14:05:38 +09:00
mikeurbach cac8d4d939
[MSFT] Fix getNearestFreeInColumn logic. (#2273)
This needs to check the available location against the target
y value. Adds a test that was reliably reproducing the issue.
2021-12-01 19:43:25 -07:00
Chris Lattner 56bad77bd2 [ExportVerilog] Try harder to synthesize temp names for extracts.
Before we'd generate:

wire [25:0] _T_55 = mem_reg_wdata[63:38];
wire [1:0] _T_56 = mem_reg_wdata[39:38];
... {_T_55 == 26'h0 | _T_55 == 26'h1 ? |_T_56 : &_T_55 | _T_55 == 26'h3FFFFFE ?

now we generate:

wire [25:0] _mem_reg_wdata_63to38 = mem_reg_wdata[63:38];
wire [1:0] _mem_reg_wdata_39to38 = mem_reg_wdata[39:38];
 wire [39:0] _T_45 = mem_ctrl_jalr ? {_mem_reg_wdata_63to38 == 26'h0 | _mem_reg_wdata_63to38 == 26'h1 ?
                     |_mem_reg_wdata_39to38 : &_mem_reg_wdata_63to38 | _mem_reg_wdata_63to38 == 26'h3FFFFFE ?
2021-12-01 17:51:11 -08:00
Chris Lattner 94178f0e2d [ExportVerilog] Try harder to make synthesized wire names prettier, part 1
When creating a temporary for a verbatim expr that looks like a macro, use
it as the basename for the temporary, instead of _T.  This produces verilog
that looks like this:

        _RANDOM = `RANDOM;      // Rocket.scala:115:20
        ex_ctrl_legal = _RANDOM[0];     // Rocket.scala:115:20
        ex_ctrl_fp = _RANDOM[1];        // Rocket.scala:115:20
        ex_ctrl_branch = _RANDOM[2];    // Rocket.scala:115:20
        ex_ctrl_jal = _RANDOM[3];       // Rocket.scala:115:20
        ex_ctrl_jalr = _RANDOM[4];      // Rocket.scala:115:20
        ex_ctrl_rxs2 = _RANDOM[5];      // Rocket.scala:115:20
        ex_ctrl_rxs1 = _RANDOM[6];      // Rocket.scala:115:20

instead of:

        _T_128 = `RANDOM;       // Rocket.scala:115:20
        ex_ctrl_legal = _T_128[0];      // Rocket.scala:115:20
        ex_ctrl_fp = _T_128[1]; // Rocket.scala:115:20
        ex_ctrl_branch = _T_128[2];     // Rocket.scala:115:20
        ex_ctrl_jal = _T_128[3];        // Rocket.scala:115:20
        ex_ctrl_jalr = _T_128[4];       // Rocket.scala:115:20
        ex_ctrl_rxs2 = _T_128[5];       // Rocket.scala:115:20
        ex_ctrl_rxs1 = _T_128[6];       // Rocket.scala:115:20
2021-12-01 16:53:49 -08:00
Chris Lattner 9fd68bc7f7 [Handshake] Remove unused "operands" signifier from ODS.
This silences some warnings in ODS generated code.
2021-12-01 15:53:51 -08:00
Schuyler Eldridge 6479b4e712
[FIRRTL] Work Around Zero Width APInt Sign Extend Issues (#2268)
Fix a bug where certain folds involving zero width constants would cause
assertions to fire.  This stems from APInt using sign extension methods
which bottom out in assertion failures and discussion upstream about
what/how to define sign extension on zero-width things.  To work around
this, this change uses zero extension methods if a zero-width APInt
needs to be sign extended.

Fix a bug in IMCP using sign extension of an APInt.

Extend existing canonicalization tests to also check situations
involving folds of signed zero-width constants.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-01 18:36:26 -05:00
Schuyler Eldridge 033486f0c6
[Support] Add APInt Utilities, NFC
Add utilities for working around limitations of APInt/APSInt and sign
extension.  This is done because many of the upstream methods hit
assertions on zero-width signed values.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-01 18:05:24 -05:00
Schuyler Eldridge d4f7e571ba
[FIRRTL] Whitespace cleanup in test, NFC
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
2021-12-01 16:24:16 -05:00
Andrew Lenharth e45746b063 [NFC] unused decl 2021-12-01 15:16:01 -06:00
Andrew Lenharth 2be4ff9ba7 [NFC] Annotation nla helper can use stringref at a couple points. 2021-12-01 14:50:30 -06:00
Andrew Lenharth 729d521b7f [NFC] add nicer builders to chirtl mems 2021-12-01 14:40:56 -06:00
Fabian Schuiki f646974aa6
[ExportVerilog] Fix bind port names not being legalized (#2228)
Fix an issue in `ExportVerilog` where the signal names used as port
connections in emitted `bind` statements would not use the legalized
name.

PR #2234 has moved the calls to `prepareHWModule` to a point before the
global name legalization. This now causes the temporary wires that were
introduced by that preparation to have a legalized name, which `bind`
statement emission requires for the port connections.

The remaining work is to adjust the `getNameRemotely` function which
provides the names for the bind port connections to properly see through
the IR pattern used to make output ports of the instance always connect
to a wire:

    %tmpWire = sv.wire
    %output = hw.instance ... () -> (output: i1) {doNotPrint = true}
    sv.assign %tmpWire, %output

This change updates this function to look through any `sv.assign` ops
from instance outputs to wires, and produces that wire's legalized name.

Also adds an assert to ensure that we always have a legal name to use
for the bind port connections. This used to be some optional code that
came up with a name on the spot.
2021-12-01 20:55:48 +01:00