Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL][CheckCombLoops] Cycle checking through references #4691

Closed
dtzSiFive opened this issue Feb 21, 2023 · 2 comments · Fixed by #5647
Closed

[FIRRTL][CheckCombLoops] Cycle checking through references #4691

dtzSiFive opened this issue Feb 21, 2023 · 2 comments · Fixed by #5647
Labels
FIRRTL Involving the `firrtl` dialect

Comments

@dtzSiFive
Copy link
Contributor

References can be part of cycles, consider detecting such scenarios. Tools such as verilator (--lint-only) will flag this, example included at end.

Test input:

firrtl.circuit "Ref" {
  firrtl.module private @Send(in %val: !firrtl.uint<2>, out %x: !firrtl.ref<uint<2>>) {
    %ref_val = firrtl.ref.send %val : !firrtl.uint<2>
    firrtl.strictconnect %x, %ref_val : !firrtl.ref<uint<2>>
  }

  firrtl.module @Ref(out %x : !firrtl.uint<2>) {
    %sub_val, %sub_x = firrtl.instance sub @Send(in val: !firrtl.uint<2>, out x: !firrtl.ref<uint<2>>)
    %res = firrtl.ref.resolve %sub_x : !firrtl.ref<uint<2>>
    firrtl.connect %sub_val, %res : !firrtl.uint<2>, !firrtl.uint<2>
    firrtl.strictconnect %x, %sub_val : !firrtl.uint<2>
  }
}

Current output:

// Generated by CIRCT 1.31.0g20230217_f4a1235
module Send(	// ref-cycle.mlir:2:3
  input [1:0] val	// ref-cycle.mlir:2:34
);

endmodule

module Ref(	// ref-cycle.mlir:7:3
  output [1:0] x	// ref-cycle.mlir:7:26
);

  Send sub (	// ref-cycle.mlir:8:24
    .val (Ref.sub.val)	// ref-cycle.mlir:9:12, :10:5
  );
  assign x = Ref.sub.val;	// ref-cycle.mlir:7:3, :9:12, :10:5
endmodule

Verilator complaint, as mentioned (5.006):

%Error: ref-cycle.sv:13:6: Wire inputs its own output, creating circular logic (wire x=x)
   13 |     .val (Ref.sub.val)  
      |      ^~~
%Warning-UNOPTFLAT: ref-cycle.sv:3:15: Signal unoptimizable: Circular combinational logic: 'Ref.__Vcellinp__sub__val'
    3 |   input [1:0] val  
      |               ^~~
                    ... For warning description see https://verilator.org/warn/UNOPTFLAT?v=5.006
                    ... Use "/* verilator lint_off UNOPTFLAT */" and lint_on around source to disable this message.
                    ref-cycle.sv:3:15:      Example path: Ref.__Vcellinp__sub__val
                    ref-cycle.sv:13:6:      Example path: ASSIGNW
                    ref-cycle.sv:3:15:      Example path: Ref.__Vcellinp__sub__val
%Error: Exiting due to 1 error(s), 1 warning(s)
@dtzSiFive dtzSiFive added the FIRRTL Involving the `firrtl` dialect label Feb 21, 2023
@dtzSiFive
Copy link
Contributor Author

Updated snippet for testing on recent firtool (1.42.0 presently):

firrtl.circuit "Ref" {
  firrtl.module private @Send(in %val: !firrtl.uint<2>, out %x: !firrtl.probe<uint<2>>) {
    %ref_val = firrtl.ref.send %val : !firrtl.uint<2>
    firrtl.ref.define %x, %ref_val : !firrtl.probe<uint<2>>
  }

  firrtl.module @Ref(out %x : !firrtl.uint<2>) {
    %sub_val, %sub_x = firrtl.instance sub @Send(in val: !firrtl.uint<2>, out x: !firrtl.probe<uint<2>>)
    %res = firrtl.ref.resolve %sub_x : !firrtl.probe<uint<2>>
    firrtl.connect %sub_val, %res : !firrtl.uint<2>, !firrtl.uint<2>
    firrtl.strictconnect %x, %sub_val : !firrtl.uint<2>
  }
}

@dtzSiFive
Copy link
Contributor Author

Also should be sure to support:

  • Forceable ref result
  • ref.sub
  • refcast
  • ref-only loops

Working on this presently.

prithayan added a commit that referenced this issue Jan 19, 2024
This is a re-write of comb loop detection pass. It simplifies the pass to use DFS
 over a reaching definitions graph to detect comb loops in the IR.
Also adds support for RefTypes and fixes few bugs.
Fixes: #4691, #5462, #6587
cepheus69 added a commit to cepheus69/circt that referenced this issue Feb 27, 2024
1. Modify the definition of MooreToCore: The original was transfering moore to HW/Comb/LLHD, and
change it to transfer moore to HW/Comb/Seq now. The dependent dialects of the pass has been
already changed.
2. Add new interface for MooreToCore to expose for other driver tools to use the pass.
3. Complement types definitions in related tablegen file.

[Moore][MooreToCore] Fix some comments and problem after merging.

[Moore] Adjust the portOp, instanceOp definition with its generation

[Moore] Remove duplicate continuous assignment op

[Moore] Imporove description of each Moore operation in tablegen definition

[Moore] Support and add new test sections for converting related signed operations.

[MooreToCore][NFC] Fix the missing configuration in relating CMake file and correct some description

[Moore][NFC] Remove extra auto-added header declarations in moore framework

[ESI][Runtime][NFC] Minor refactor and cleanup (#6539)

- Moves classes around.
- Changes Type ownership.
- Cleans up #includes.
- Adds documentation

[ESI][Runtime] Add design hierarchy printing to esiquery (#6540)

And add a test as well.

[ESI][Runtime] Add type serialization support to Python bindings (#6541)

- Mirror the C++ design and type hierarchy.
- Add [de-]serialization support to/from Python objects.
- Add support for `void` type.

[PyCDE] Fix ESI integration tests (#6542)

Adapting to new runtime API.

[FIRRTL][LowerSigs][NFC] Simplify Case to Default.

Being explicit has benefits too, but simplify.

[FIRRTL] Remove unnecessary template keyword, NFC

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[PyCDE] Fix ESI service implementations (#6545)

Repair the ability to implement services in PyCDE. I broke this some
months ago and never got around to fixing it.

[support] Add debug header print helpers

Add two helpers for printing boilerplate that is commonly used when
generating LLVM debugging information for passes.  This will be used to
replace this boilerplate in passes in a future commit.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] Convert pass debug info to use utils

Change boilerplate llvm::dbgs() printing to use a new utility.  This
avoids duplication of the same boilerplate in every pass.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[CI] Fix release asset upload job permissions, support manual runs. (#6547)

Give write permissions to jobs that upload release assets.
Set the release-tag explicitly so the upload action knows where to upload on manual runs (on a tag). When triggered by a release this isn't needed but doesn't hurt.

Bump the upload-release-assets action v2 -> v3: https://github.com/AButler/upload-release-assets/releases/tag/v3.0
(This is needed for the release-tag support, also fixes a warning about node version).

[CI] buildAndTestWindows: Set "write" permission to unbreak for now.

[CI] Bump runner for windows release artifacts 2019 -> 2022. (#6548)

Match what we use in CI.

[LowerSignatures] Fix instance locations (#6550)

Fix an issue in the `LowerSignatures` pass where instances would discard
their location and inherit the parent module's location instead.

Fixes #6535.

[FIRRTL] Whitespace .td cleanup, NFC

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] Add util to maybe get an attribute

Add a utility in FIRRTLOps.cpp for getting an optional attribute from an
array of named attributes.  Previously, utilities were only provided for
getting mandatory attributes.

Change existing utilities to use this new utility under the hood.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL][NFC] Drop semicolon, fix warning.

[CI] Add options to control whether workflow_dispatch assert + build mode. (#6549)

[NFC] fix warnings in tcl bindings

[Docs] fix typos in Dialects/ (#6555)

[NFC][FIRRTL] Remove whitespace from test

[FIRRTL] Move intrinsics into their own tablegen file

[NFC] Move off of deprecated stringref apis

[NFC] Move off of deprecated stringref apis

[FIRRTL] Update InstanceGraph on erase in LowerClasses. (#6558)

We intended to keep the InstanceGraph up to date, but when we
completely erase FIRRTL ClassOps, we were not removing them from the
InstanceGraph. This can lead to invalid memory accesses, which I
noticed with ASAN while using the InstanceGraph in a pass downstream
of this.

[FIRRTL] Fix the lowering of internal paths in lower signatures (#6556)

Dummy internal paths must be added to expanded ports.

[HW] Select the better name when dropping wires (#6559)

[NFC] Bump LLVM over mnemonic change (#6563)

[Sim] Introduce the rationale for the `sim` dialect (#6536)

[FIRRTL][NFC] Add a test for plusarg lowering

[Sim] Initial implementation of the `sim` dialect (#6561)

[FIRRTL] Miscellaneous Whitespace Cleanup, NFC

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[Python] Reduce wheel matrix and scheduled frequency.

We are running out of space in PyPI, so this removes support for
versions Python 3.9 and Python 3.11, which are not used by anyone in
the community at the moment. It also reduces the scheduled frequency
of dev wheel builds from nightly to every Monday at noon UTC.

[NFC] Fix warnings from missing include in ESI runtime. (#6567)

[ESI] Fix WrapValidReadyOp folder bug

Worked until now, but the upcoming llvm bump doesn't like it.

[OM][Python] Fix typehint in integration test

In Python3.8, must use `typing.Dict` to subscript types.

[FIRRTL] Innocuous change to mitigate #6513 (#6571)

This change seems to make the test problem go away.

[FIRRTL] Update test from groups to layers, NFC

Change a test to be called "layers.fir" and not "groups.fir".  Change it
internally to use layers (the final name of the feature) and not
groups (the early version of the feature).

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[Arc] Use CallOp instead of latency 0 StateOp (#6560)

This simplifies the `arc.state` operation by always requiring latency > 0 and uses `arc.call` operations instead of the latency 0 states.

Bump LLVM (#6566)

[NFC] Fix some types on loops for getNumPorts.  Thanks to @Tang-Haojin.  May fix windows build.

[NFC] try cleaner fix to windows issue (#6573)

[FSM] Add CAPI dependency on conversion pass header generation. (#6572)

When the conversion from FSM to SV was added to the CAPI, an include
of the circt/Conversion/Passes.h header was included for the pass
registration function. But nothing guarantees the generated header
that is included by Passes.h actually exists.

We have seen intermittent failures from it not existing, for example:
https://github.com/llvm/circt/actions/runs/7474242994/job/20339994666

This adds a DEPENDS on the target that generates the missing header,
so it will be available.

[NFC] fix warning

[Arc] StateOp: latency instead of lat in assembly format (#6562)

Spelling out latency should make it easier to understand what this
attribute means.

[Arc] Dedup: Fix use after free (#6568)

[NFC][Seq] Rename the ClockDivider to ClockDividerOp

[NFC][FIRRTL] Move the clock gate intrinsic to the instrinsics file

[NFC] llvm bump

[NFC] Convert getPortAttributes back to ArrayRef (#6531)

Several functions had to be pessimized when converting to module type. Start moving them back to more optimized forms.

[FIRRTL] Add Layer Association to Probes

Add support for representing an optional layer in each probe type.  This
only handles storage and MLIR printing/parsing.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[PyCDE] Fix ESI integration tests

broken image src's fixed (#6578)

Update FSMToSV summary (#6580)

[ESI][Runtime] Add a utility wrap a command with a cosim (#6579)

The 'esi-cosim' script starts a simulation, waits for it to start, then
executes the 'inner' command. When the specified command exits, it kills
the simulation. Very helpful for debugging designs via ESI cosimulation.

Only supports Verilator currently.

[NFC][Seq] Use the rewriter in the clock gate lowering

[NFC][Seq] Do not update temporary ops with names

[ESI] Fix integration tests (missing file)

[NFC] LLVM bump

[PyCDE] New ignore for dev-convient soft link

[ESI] Cosim MMIO: detect too many read/write responses

If the simulation attempts to send more responses than there are
outstanding requests, log an error.

[Builds] Revert back to small runners (#6586)

Self-hosted runners were disabled in our repo, so none of our builds are
being picked up. Going back to the slow ones (with less disk space)
while we figure out the situation.

Revert "[Builds] Revert back to small runners (#6586)"

This reverts commit 34489a17f06df6af09219fd5527c16fbcba556d6.

[ESI] Introduce MMIO std service (#6584)

Currently, can only request a single 32-bit register. In the future,
we'll add the ability to request structs of config/status registers.

[ESI] Lower manifest op to a ROM (#6585)

Since ESI hardware embeds the manifest, lower the zlib-compressed version of it to a module to be instantiated by a BSP.

[FIRRTL][CAPI] Expose `foldFlow` function

[PyCDE] Verify the system after finishing generation

Helps improve debuggablity of the design.

[PyCDE] Fix esi_ram integration test

Changed 'write' to 'req' on the CIRCT side without updating the PyCDE
side. Fixed.

[PyCDE] Support for ESI MMIO service -- read side only (#6590)

Add support for the MMIO ESI service to PyCDE. Most implementations of this service will embed the compressed manifest, which this PR is intended to support. So let's just expose the read service for now.

[HW] Replace func.func with hw.module in HW/errors.mlir, NFC

[NFCI] Don't clone arith dialect attrs into comb.  Comb doesn't know about these.

[PyCDE] Expose input ports, rename output getter, fix name issue

[FIRRTL] Naming bounce wires properly (#6594)

LowerSignatures creates anonymous bound wires as a place holder but these didn't get proper names from ports.

[PyCDE] Fix service generator warning (#6595)

Fix warning about holding on to MLIR ops. Service generator instances
get held through a C++/Python interaction barrier, unlike regular
instances. So the member variable `inst` (which points to the MLIR
instance operation) has to be cleared before attempting to clear the
live operations.

[ESI] MMIO based manifest cosim support (#6592)

Adds a MMIO service implementation for CosimBSP. Doesn't actually fulfill any service requests, just exposes a header (magic number, version, ptr to the manifest) and the system manifest. Implemented in PyCDE as part of the Cosim BSP.

Uses the AXI MMIO cosimulation DPI module so this implementation is likely to work on any platform which uses AXI-lite for device MMIO (e.g. any XRT platform, like the U250 on Azure's NP instances).

[CombFolds] Flatten operands all at once (#6593)

This fixes O(N^2) compile time regression in OrOp(and other logical operations) canonicalizers. Currently tryFlattenOperands flattens operands by only one level, e.g:

Or(a, Or(b, Or(c, d)))
->
Or(a, b, Or(c, d))
->
Or(a, b, c, d)

Problematically this canonicalizer visits operands until a first operand we can flatten every time so this is problematic when chains is really long. This PR fixes the issue by flattening all operands recursively from a root all at once. Also flatteing is moved to a start of canonicalizations so that other canonicalizations are only applied to root operations. This PR should be essentially NFC but there are several differences in verilog files since canonicalizations are applied in a different order and are not canonical.

Benchmark for one of the largest internal cores:
```
Before:
  389.7719 (  9.6%)  341.5307 ( 29.6%)    Canonicalizer
  4077.4128 (100.0%)  1153.8959 (100.0%)  Total

After:
   43.4950 (  1.3%)    1.7205 (  0.2%)    Canonicalizer
  3350.8652 (100.0%)  803.7959 (100.0%)  Total
```

Fix one of the problems of https://github.com/llvm/circt/issues/6587

[CombFolds] Fix a typo in a comment, NFC

bump llvm submodule to tip of main (103fa3250c46) (#6589)

[Comb] Handle type aliases in `comb.concat` (#6588)

* [Comb] Handle type aliases in `comb.concat`

* [Comb] Add regression test (#5772)

* [Comb] Adhere to convention

* [Comb] Test for presence of `comb.concat` in output

[CheckCombLoops] Refactor comb loop detection pass (#5647)

This is a re-write of comb loop detection pass. It simplifies the pass to use DFS
 over a reaching definitions graph to detect comb loops in the IR.
Also adds support for RefTypes and fixes few bugs.
Fixes: #4691, #5462, #6587

[FIRRTL][FIRParser] Tweak error for Probe<T> where T isn't base.

Probe<Integer> shouldn't complain about nested "reference" types.

[ESI] XRT support for MMIO based manifest fetching (#6596)

Xilinx Runtime Shell (XRT) support for exposing the MMIO header and
manifest over MMIO. XRT is the platform used by Azure's NP instances
which have access to attached U250 FPGAs. Works in both hw_emu and real
hardware.

[PyCDE] Fixing tests broken by XRT merge

Oops... didn't run these tests!

[PyCDE] Add ESI metadata to modules (#6597)

Users can either explicitly specify metadata or it can be automatically generated.

[PyCDE] Remove a bunch of old cruft

A lot of this functionality was replaced by the ESI runtime.

[Ibistool] Replace wide include with something more targetted

Avoids introducing implicit dependencies on header files which may or
may not be generated. (GPU dialect was the specific issue.)

[PyCDE] Fixing ESI manifest test for builders without GitPython

FileCheck line assumed that repo and commitHash were filled in, which
they wouldn't be if GitPython isn't installed.

[FIRRTL] Create debug info scopes for inlined modules (#6512)

When inlining and flattening FIRRTL instances, create `dbg.scope`
operations to track information about the original hierarchy in the
debug info.

To do this, the FIRRTL inliner now creates a `dbg.scope` op for every
inlined instance. When renaming the operations in the inlined module's
body, debug operations are assigned this scope (unless they already had
a scope assigned). This retains information about the original
hierarchy.

Unfortunately, this approach currently only works for debug variables
and scopes. Instances have no `scope` operand where a parent scope can
be annotated. As a result, instance ops whose parent module got inlined
do not properly track their original scope. This limitation will go away
in the future once we are either able to interact with and modify the
implicit scope created by instances, or instances scopes get passed in
as explicit operands.

[NFCI] Test cases are invalid if initialization check happens early

[FIRRTL] Make the Field Source analysis more robust to newer ops

[Seq] Add a representation for clock inverters (#6575)

[NFC] bump llvm

[FIRRTL][ExtractInstances] Clear entire state on pass invocation (#6599)

Clear 'dutModuleNames' and 'dutPrefix' along with the other members before running the pass.

[NFC] llvm bump

[NFC][CheckCombLoops] Allow UnrealizedConversionCast to close loops

[NFC] Remove extraneous dump

[FIRRTL] Initial support for classes and objects in Dedup. (#6582)

This adds initial support for Dedup to handle deduping classes and
objects. For the most part, this amounts to ensuring core
functionality is implemented in terms of the FModuleLike and
FInstanceLike interfaces, which classes and objects implement,
respectively.

There are a couple places where some specific logic is added for
objects, similar to the specific logic that was already there for
instance ops.

Finally, a small change related to paths is needed in
LowerClasses. There is some logic there that confirms a hierarchical
path's root module is contained within the path's owning module. In
the case of deduping hierarchical paths, an extra layer of hierarchy
is added to hierpath ops, which breaks this logic. A new condition is
added, and if the owning module is anywhere in the hierarchical path,
it is used as the root module.

An end-to-end firtool test is added to demonstrate that classes and
objects dedup, and the final paths are valid in both the local and
hierarchical cases.

This initial support is capable of generating valid paths, but does
not actually confirm that paths which dedup point to entities that
dedup. Comments have been left, and a ticket opened to address this:
https://github.com/llvm/circt/issues/6583.

This initial support does not handle references to objects in class
ports, in the case the objects' classes are deduped. A ticket has also
been opened to address this:
https://github.com/llvm/circt/issues/6603

[NFC] remove some warnings

[NFC] functions should be file local

[NFC] bump llvm

[PyCDE] Fix import hw modules (#6130) (#6605)

Fixes #6130.

[NFC] Rename confusing api.

.get() on a type should construct a new type.  BaseAlias was using this function to ALSO return the wrapped type.  Rename the accessor to not conflict with the factory.

[NFC] Rename Python wheel job name from Nightly to Weekly.

We have changed the job to run weekly to avoid running out of space in
PyPI. This just updates the job name to reflect that.

[HW to BTOR2] Add support for nested Wires and Compreg (#6602)

[NFC] Linewrap fixes

[FIRRTL]  CheckCombCycles ignores foriegn ops. (#6612)

Casts should be cast like, but foriegn ops are ignored.  Combinatorial cycles are only defined for FIRRTL.
Make one remark per module if there is a potentially missed loop.

[FIRRTL] Fix shr(<zero-width>, 0) folder crash

Fix a bug in the `shr` folder where a shift by zero of a zero-width value
would cause a crash.  As of FIRRTL 3.3.0 the result width is size 1.  If
this is folded to the input, then it causes a crash because types no
longer match.  Disable this fold, as we do with mux in the same situation.

h/t @fabianschuiki for suggesting the fix.

Fixes #6608.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[NFC][FIRRTL] Canonicalize away empty probes (#6617)

[NFC][InstanceGraph] Add a method to drop the last element of an instance path

[FIRRTL] Add parser/emitter support for layer-associated probes (#6552)

* [FIRRTL] Parse Layer-Associated Probes

Add support to the FIRRTL parse to handle probes associated with layers.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* [FIRRTL] Emit Layer-associated Probes

Add emitter support for layer-associated probes.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* fixup! [FIRRTL] Parse Layer-Associated Probes

* Add a fir version check for colored probes

---------

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Co-authored-by: Robert Young <rwy0717@gmail.com>

[scf-to-calyx]delete identifiers. (#6611)

* delete literal identifier.

* Update include/circt/Conversion/SCFToCalyx.h

Co-authored-by: Chris Gyurgyik <Gyurgyikcp@gmail.com>

* use clang-format.

* add identifiers in CalyxOps.h

---------

Co-authored-by: Chris Gyurgyik <Gyurgyikcp@gmail.com>

[HW] Remove the cache-based lookup method from HWInstanceLike (#6622)

Since it is only used in ExportVerilog, there is no need to expose it to all dialects.

[FIRRTL] Use isX intrinsic for trueOrIsX verif statements. (#6625)

Prefer intrinsic over verbatim verilog.

[ImportVerilog] Add Slang frontend dependency (#6620)

This is the first PR in a longer chain that adds basic SV support to
CIRCT.

Add the Slang Verilog frontend as a CIRCT dependency. This will be the
foundation for CIRCT's Verilog parsing, elaboration, type checking, and
lowering to the core dialects. By default, Slang is built as a static
library from scratch, which is then linked into the new `ImportVerilog`
conversion. Alternatively, CIRCT can also be linked against a local
Slang installation provided by the system.

Add the `ImportVerilog` conversion library. This library statically
links in the Slang dependency and wraps it in an exception-safe,
LLVM-style API. Currently this only consists of the `getSlangVersion`
function and the necessary linking flags to get it to link statically
against Slang.

Add the `circt-verilog` tool, which will provide a fully-flegded
interface to the new `ImportVerilog` library. Later on we'll also add an
MLIR translation library for single-file SV import. But in general, SV
builds take a lot of command line options (macros, search paths, etc.)
and multiple input files, which is why we have a dedicated tool. All the
tool does at the moment is print the linked Slang version. More to come.

Note that this intentionally links against **version 3** of Slang. Newer
versions are available -- 4 and 5 as of this commit -- but they rely on
fairly new C++ compiler features that didn't work out of the box in our
CI images. We'll eventually want to upgrade, but for now Slang 3 is
sufficient to get the ball rolling.

See https://github.com/MikePopoloski/slang for details on Slang.

Co-authored-by: ShiZuoye <albertethon@163.com>
Co-authored-by: hunterzju <hunter_ht@zju.edu.cn>
Co-authored-by: 孙海龙 <hailong.sun@terapines.com>

[HW][CAPI] Add instance graph C-API for iterating modules

[ImportVerilog] Add missing MSVC option for Slang

Add a missing `/EHsc` option to Slang builds using MSVC.

[ImportVerilog] Disable Slang frontend in MSVC CI builds

MSVC fails to build the Slang 3 dependency due to issues in Slang's
`MathUtils.h`. (The file is missing an `#include <limits>`.) Since we
are planning to upgrade to Slang 4 or 5 anyway, don't bother setting up
a full workaround for this issue yet and instead just disable Slang in
MSVC CI builds and installs. Once the Verilog frontend integration
matures and we update Slang, we can go back in and enable MSVC builds
again.

[HW][CAPI] Remove use of designated initializers in new CAPI test.

These are a C++20 feature, which is not supported yet. This seemed to
have slipped through pull request CI, but is caught by msvc on main.

While fixing this test, this also squashed a warning related to an
assertion by ensuring the literal 5 is 5UL.

[NFC][InstantePath] Add equality comparison

[FIRParser] Allow assert and assume stmt to have a format string (#6621)

This implements chipsalliance/firrtl-spec#166

[ESI][Runtime] Support Questa (#6630)

Adds `--sim questa` option to `esi-cosim.py`. Turns out the new cosim
stuff didn't work with Questa. Fixed that too.

[Python] Register the comb dialect lowerings

[CIRCT] Add TypedArrayRefAttrBase

Add the typed equivalent of ArrayRefAttr.  This provides the same benefits
of ArrayRefAttr (you get back an ArrayRef and can use this directly as
opposed to doing checks of the, possibly null, ArrayAttr).

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[Comb] Fold mux if both arms are same constant

If both arms of a `comb.mux` evaluate to the same constant, fold the mux
to that same constant. Right now this is only done if the SSA value is
identical. Some constant propagation algorithms will call `fold` to
propagate lattice constants, which isn't covered by just comparing SSA
values.

Short: also check adaptor's true and false value for folding, not just
the op's true and false value.

Hack: Run CI on GitHub runners

WIP: [ImportVerilog] Slang frontend integration

Co-authored-by: Fabian Schuiki <fabian@schuiki.ch>
Co-authored-by: ShiZuoye <albertethon@163.com>
Co-authored-by: hunterzju <hunter_ht@zju.edu.cn>
Co-authored-by: 孙海龙 <hailong.sun@terapines.com>

* [ImportVerilog] This PR is aimed at showing the features that Moore supports now. (#26)

* WIP: [ImportVerilog] Slang frontend integration

* [ImportVerilog] Add some new types which are equivalent to PredefinedIntegerType in slang::ast.

* [ImportVerilog] Add some new test cases to detect.

* [ImportVerilog] Add support for the RealType that is equivalent to FloatingType from slang::ast namespace.

* [ImportVerilog] Add some new test cases for RealType.

* [ImportVerilog] Add support for the moore::UnpackedUnsizedDim and moore::UnpackedRangeDim

* [ImportVerilog] Add some new test cases and adjust the order of CHECK-LABEL in this file.

* Add support for the moore::UnpackedAssocDim and moore::UnpackedQueueDim.

* Add some new test cases for UnpackedAssocDim and UnpackedQueueDim.

* Adjust the order of test cases to pass FileCheck correctly.

* [ImportVerilog] Add support for the NetType.

* [ImportVerilog] Add new test cases for the NetType.

* [ImportVerilog] Add support for the moore::EnumType, but there are no related test cases.

* [ImportVerilog] Some unnecessary semicolons were removed.

* [ImportVerilog] The handling of some IntTypes was deleted due to an earlier error and is now restored.

* [ImportVerilog] Able to handle 32-bit assignment Op.

* [ImportVerilog] Able to handle 32-bit assignment Op and handle single blocking assignment in always_comb block.

* [ImportVerilog] About the definition of Moore::AlwaysCombOp.

* [ImportVerilog] WIP:Can't handle cases where the right side of an AssignOp is a variable.

* [ImportVerilog] Add support for the moore::AlwaysCombOp and a new file called Statement.cpp is created, which is used for slang statement conversion.

* [ImportVerilog] Create a new file called Statement.cpp which is related to Slang statement conversion.

* [ImportVerilog] Change break in the case statement to call mlir::emitError().

* [ImportVerilog] There are tweaks to the framework for handling expressions. Andadd new supported for the handling varibles and nets.

* [ImportVerilog] Add a new file to handle expression specially.

* [ImportVerilog] Add a new line at the end of Expression.cpp.

* [ImportVerilog] Add a new line at the end of Expression.cpp.

* [ImportVerilog] Tweak a little detail.

* [ImportVerilog] Support continuous assignment, but rhs can not a variable.

* [ImportVerilog] Add support for the moore::IfOp.

* [ImportVerilog] Add two new files for slang statements and expressions conversion.  And now can support conversion of the alwaysCombOp, initialOp, and ifOp.

* Update CMakeLists.txt

* Update MooreOps.cpp

* Update Structure.cpp

* Update Structure.cpp

* [ImportVerilog] A lot of things, review the details at the framework branch.

* [ImportVerilog] Add EqualityOp to handle four equal operator.

* [ImportVerilog] Implement the body of the visitBinaryOp func for choosing which op will be used.

* [ImportVerilog] Implement the body of the visitSignalEvent func.

* [ImportVerilog] Add moore::IfOp and modify the type of AssignOp.

* [ImportVerilog] Support the relational and shift operators and introduce the scf::IfOp.

* [ImportVerilog] Add the moore::LogicalOp to handle four logical operators.

* [ImportVerilog] About the unaryOp, binaryOp, netOp and conversionOp. And add the test.sv file for testing existing feature.

* [ImportVerilog] Support AddOp, MulOp and improved assignment Op (#27)

* [ImportVerilog] add PCassignOp and fix test.sv

 Create pcassign op when expr is Procedural continuals
 assignment.
 Tweak `test.sv` to pass the FileCheck.
 Clean up unnecessary functions.

* [ImportVerilog] Support AddOp and MulOp

AddOp and MulOp support variadic operands.
The element of operands should be same.

WIP: Add procedure tests, add missing bail-on-error

WIP: Remove lvalue/rvalue type distinction

WIP: Clean up ops and make tests more focused

WIP: Minor restructuring

WIP: Improve cast op and add more tests

WIP: Add more dedicated binary ops, add wildcard eq/neq, refactor

[Build] Fix building problem after merging the content from force-updates

[Moore][Transform] Introduce new passes into new Moore transform directory

[MooreToCore] Adding support for the conversion of SVModuleOp / NetOp / VariableOp / CAssignOp

[MooreToCore] Align name of all conversion patterns

[MooreToCore] Able to convert unaryOp to parityOp.

[HW] Make HWInstanceLike agnostic of the number of targets (#6623)

[ESI] Simplify services by standardizing on `to_client` ports (#6633)

Having a direction on the service ports which just reversed the bundle directions was far too complicated. Standardize on the service always packing the bundle and sending it to the client. Standardizing that way has the additional benefit that this was already the canonical form which all the service generators saw. Remove all to_client verbiage from the IR.

[PyCDE] Adapt to new to_client style services (#6634)

Turns out the default style PyCDE selected was the non-canonical one --
to_server -- instead of to_client. Standardize on to_client like CIRCT just did.

[ESI][Runtime] Small improvements to cosim runner

- Add timescale to sv driver
- Define SIMULATION macro

[PyCDE] Add casts, fix clock enable

- Adds the bitcast method to Signal, allowing any Signal to be converted
to any type of the same width.
- Add to `to_bit` method to ClockType to change it to an i1.
- Remove broken ce arg.

[PyCDE] Support ESI's FuncService (#6636)

Adds support for implementing function calls in PyCDE.

CMake: Fetch slang via git to workaround version check bug. (#6640)

Fixes #6639.

[FIRRTL] Prerequisites for Layer-Associated Probes (#6574)

Miscellaneous fixes to get layer-associated probes working.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Co-authored-by: Robert Young <rwy0717@gmail.com>

[FIRRTL] Add enabled layers to FModuleOp (#6627)

* [FIRRTL] Add layer colors to FModuleOp

Add the ability for FIRRTL module to enable layers.  This will have the
effect of allowing a module to interact with colored probes of instances
that it instantiates.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* [FIRRTL] Add verification of FModuleOp layers

Extend existing verification of 'firrtl.ref.cast', 'firrtl.ref.define',
and 'firrtl.ref.resolve' to handle modules which may enable layers.

This also refactors all the layer-coloring verification into a single
function that is reused by all three operation verifiers.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* [FIRRTL] Pipeline updates for module layers

Updates to the FIRRTL pass pipeline to support modules with enabled
layers.  These do not have a real effect on the pipeline itself other than
they should be removed once all ops that rely on their existence to pass
verification are removed.  I.e., this commit only removes them in
LowerXMR.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* [FIRRTL] Parse/emit module-enabled layers

Add FIRRTL text parsing and emission for modules with layers enabled.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* Update all FModules to support enabled layers

* Add parsing/emitting for enablelayer for Intmodule/Extmodule

- improve formatting of output

---------

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Co-authored-by: Robert Young <rwy0717@gmail.com>

[HW] Lower hw.instance_choice to SV (#6624)

[NFC] Remove DEBUG_TYPE from a file which does not use it

[ESI][Runtime] Convert Type& to Type* (#6644)

References were too complex to bother.

[ESI][Runtime][NFC] Move `requestChannelsFor` into accelerator connection (#6646)

Everything needs channels, not just custom services.

[ESI][Runtime] Add the notion of a Context (#6647)

ESI's runtime context is exactly the same as an MLIR context. It owns
everything which essentially wants to be global.

[ESI][Runtime][NFC] Switch from ptr,size to MessageData (#6648)

Not a big improvement, but will make function calls easier.

[NFC][Seq] Propagate name hints from clock inverters

Fix compiler warnings re: InstanceChoice

Fix a few compilation warnings that sneaked through related to the
Instance Choice op.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[NFC] Fix build under linux (clang-15)

[FIRRTL] SHR canonicalizers of uninferred width could produce a different width before and after width inference when shifting by zero

Expand testcases from https://github.com/llvm/circt/issues/6608 to include uninferred widths.

[ESI][Runtime] Return a py::object so we can return None on failure

Fixes #6653 and adds functionality to test the fix.

Fix some issues in the LayerBlockOp verifier (#6654)

When checking the ops under a layerblock:

- When checking if an operand refers to a value defined outside the layerblock,
  we need to check if the layerblock is an ancestor of the value's definition,
  not the more strict "direct parent of definition" check. This check better
  handles blocks (such as when ops) nested under a layerblock.
- When checking if an operand is valid, we shouldn't early-return, which would
  disables all remaining verification for the op.
- Explicitly allow ref.define op to define destinations outside the layerblock
  op. It is the only connect-like op that is allowed to do so.
- Use WalkResult::interrupt() to signal failure.

[NFC][FIRRTL] Expose hasZeroBitWidth to external users

[FIRRTL] Use set-based logic to test for layer compatibility (#6643)

[FIRRTL] Intrinsics: Fix mistakenly preserved analyses. (#6666)

Conversions weren't actually counted, fix.

[OM] Remove Symbol trait from ClassFieldLikes. (#6665)

Since 5fb4cf5fd3a3dcaa05b43e340120ce9c3a75f597, ClassLikes are not
SymbolTables, due to the usual reasons that led to us inventing
InnerSymbolTables. However, that change left the Symbol trait on
ClassFieldLikes, which is not legal. We need to fix this, and stop
using Symbols in illegal locations.

This change removes the Symbol trait, and includes some minor juggling
so the current implementation can continue working with minimal
changes.

This renames the attribute name from sym_name to name and adds an
interface method to ClassFieldLike called getNameAttr. Together, these
changes align the accessor for this field to match the previous
getNameAttr accessor on Symbols.

This also updates a couple call sites that directly called
getSymNameAttr to use getNameAttr like the other call sites.

This updates the ObjectFieldOp verifier, which already has to linearly
walk a ClassLike's fields, to stop walking SymbolOpInterface and
instead directly walk ClassFieldLikes.

The last thing is a non-change, where we keep the name attributes as
SymbolNameAttr in ODS, so the printed form still looks like a
symbol. This also aligns with the ObjectFieldOp printed form. This is
so this change can be backwards compatible in the printed IR; if we
think it is better to call a spade a spade, this can be updated to
print these field names as quoted strings, to better reflect that
they're actually just StringAttrs.

Fixes https://github.com/llvm/circt/issues/6659.

[Calyx] Make ControlOp a SymbolTable. (#6670)

* [Calyx] Make ControlOp a SymbolTable.

During the Calyx control lowering, in an intermediate state, we have
fsm.machine ops embedded within the calyx.control op. fsm.machine
defines a Symbol, so this is illegal unless calyx.control is a
SymbolTable.

We consistently hit issues when we try to nest SymbolTables, but I
don't think those will actually bite us in the Calyx lowering
pipeline. Since no one is actively maintaining this, I'm proposing to
add SymbolTable to ControlOp until we have a reason to change this, so
the IR is legal during the Calyx lowering.

Fixes https://github.com/llvm/circt/issues/6667.

[NFC][IMConstProp] Fix race condition with aggregate preservation

[IMCP] Fix a race condition regarding aggregate preservation (#6671)

rewriteModuleBody is ran parallely so we can ignore the overhead of
plain getFieldRefFromValue.

[FIRRTL] Track the enablelayers array on instance ops (#6663)

The enabled layers of a module act as requirements on the instance op--the
module may only be instantiated where the enabled layer set is in the ambient
layer set.

This PR adds verification for this rule.

Changes:
- Start tracking the required layers on instance ops.
- Verify the ambient layers of instance ops.
- Verify that the layers attribute on instance ops agrees with the referenced
  module.
- Update places where we create instance ops to pass the layers.

Add statically linked CIRCT full build to ReleaseArtifact CI (#6544)

shared build seems to require a specific version of glibc and causes a build failure of downstream tools. This PR adds -static tarball to the artifact.

[FIRRTL] chisel_{assert_assume,assume,cover,ifelsefatal} intrinsics. (#6664)

Provide intrinsics to capture what today is encoded via printf + when/stop/verif-op pattern matching and XML + JSON parameters.

Compared to assert/assume/cover FIRRTL (textual/in-the-spec) ops, these allow specifying labels that are more than identifiers and have optional compilation guards.

These map to current FIRRTL ops directly, including the various special behaviors that means in practice today (as encoded in LowerToHW):

* Assert implies companion assume emission.  Hence `chisel_assert_assume`.
* If the guard `USE_UNR_ONLY_CONSTRAINTS` is present, this companion assume is different, see: https://github.com/llvm/circt/pull/5561 .

There is no "assert" because `firrtl.assert` "concurrent" (SVA) forms always "imply" a "companion assume" and presently there is no FIRRTL dialect encoding for just a concurrent assert without this implied assume.

These are all SVA/concurrent "type".

IfElseFatal:
chisel_ifelsefatal becomes a very specific pattern of verilog that is not concurrent but is modeled that way in the IR (special "assert").
Allow guards and label on the intrinsic but they don't do anything on normal emission flow (`-emit-chisel-asserts-as-sva` will see them however so expose them).

[MSFT] Remove ChannelOp

This was unused. Fixes #6660.

[ImportVerilog] Add import options and Verilog preprocessing (#6632)

Add the `ImportVerilogOptions` struct to capture various options that
can be passed to the Slang Verilog frontend. This covers things like
include search directories, macro definitions, top modules, time scales,
and other settings commonly found in Verilog tools. The struct is
inspired by `slang::driver::Driver::Options`, but we intentionally
create our own struct such that no Slang headers leak through into CIRCT
space.

Add corresponding command line options to `circt-verilog`, and use them
to populate an `ImportVerilogOptions` struct that can be passed to
`ImportVerilog`.

Add `importVerilog` and `preprocessVerilog` functions to `ImportVerilog`
and add corresponding options to `circt-verilog` that call the
respective import function. This commit only implements preprocessing.

This commit also adds a mechanism to bridge between the diagnostic world
of Slang and MLIR (`MlirDiagnosticClient`). This allows us to emit any
Slang-generated diagnostics as regular MLIR diagnostics, alongside any
other errors or warnings the Verilog import might generate.

Add tests for the preprocessor (with include paths and macros) and the
diagnostic bridge.

Co-authored-by: Will Dietz <will.dietz@sifive.com>
Co-authored-by: ShiZuoye <albertethon@163.com>
Co-authored-by: hunterzju <hunter_ht@zju.edu.cn>
Co-authored-by: 孙海龙 <hailong.sun@terapines.com>
Co-authored-by: Will Dietz <will.dietz@sifive.com>

[circt-verilog] Add "REQUIRES: slang" to new tests

Add missing requirements on slang for recently added circt-verilog
tests. This gets my local build passing tests and may fix failures
observed on Windows CI.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

Allow propassign under layerblocks (#6656)

Also, ban capturing outer properties inside a layerblock.

[NFC] Remove test demonstrating illegal IR.

This test appears to be testing ExportVerilog for a pattern that isn't
actually legal, since a `hw.type_scope` defines a Symbol. These days,
the relevant ifndef guard is emitted as a verbatim, for the exact
reason that the `hw.type_scope` must be in the top-level module's
SymbolTable.

Fixes https://github.com/llvm/circt/issues/6669.

[Seq] Remove Symbol trait from HLMemOp. (#6676)

These are used in contexts that are not SymbolTables, so these cannot
be Symbols. It appears that nothing actually needs the Symbol
functionality, and all we need here is the string name. This removes
the Symbol trait and renames the attribute to just name.

If anything needs to reference HLMemOps symbolically, these can become
InnerSymbols, but again, I didn't see anything about that in the
codebase.

Fixes https://github.com/llvm/circt/issues/6661.

[ESI][Runtime] Adding support for FuncService (#6673)

Fleshing out support for standard services and addition to the manifest.

[FIRRTL] InferResets: verify that FART annotation is on async resets (#6674)

The FullAsynchronousResetAnnotation is used to mark an asynchronous
reset signal in a module to be used as the reset for all registers that
do not already have an async reset in this module and below in the
hierarchy. There was no error checking that this annotation was attached
to an asynchronous reset typed signal, and the pass would accidentally
try to wire whatever signal was annotated. In some cases, this could
lead to a connecting the signal to an async typed port, which would
become a type checking error in the verifier. This changes InferResets
to verify that the FART annotated signal is in fact asynchronous.

[FSM] Remove Symbol trait from InstanceOp and HWInstanceOp. (#6675)

These are used in contexts that are not SymbolTables, so these cannot
be Symbols. It appears that nothing actually needs the Symbol
functionality, and all we need here is the string name. This removes
the Symbol trait and renames the attribute to just `name`.

Fixes https://github.com/llvm/circt/issues/6668.

Co-authored-by: John Demme <john.demme@microsoft.com>

[FIRRTL][LowerXMR] Use FIRRTL 4.0 ref ABI. (#6677)

Fix support for refs into extmodule that's a public module within a separate circuit.

Prepare for upcoming FIRRTL 4.0.

ABI change not versioned so breaks compatibility.

Previous different fix: #6172 .

Tree-wide test fixes for FileCheck directive typos (#6679)

Found via the (TIL) LLVM utility llvm/utils/filecheck_lint/filecheck_lint.py.

[LowerIntrinsics] Accept EICG_wrapper without test_en, reject annos (#6678)

Accept `EICG_wrapper` extmodules without `test_en` port in
`LowerIntrinsics`, and simply leave the `test_en` unconnected in that
case. Also reject `EICG_wrapper` modules and instances if they have
annotations attached, as we currently have no mechanism to carry them
over onto the intrinsic. (Only if `--fixup-eicg-wrapper` is passed to
firtool.)

[ESI] Add some missing macOS includes

Add some missing 'sstream' includes that were need on a fresh build on
macOS.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] Emit public modules with "public" keyword

Emit public FIRRTL dialect modules with the "public" keyword.  This brings
the FIRRTL emitter further in line with FIRRTL 4.0.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[WireDFT] Disable the pass by default (#6684)

A hidden command-line argument is added to re-enable the pass on possible backports or during the next release.

[LowerToHW] Fix shr(0-bit, n) lowering (#6683)

Fix an error if a 0-bit was shifted right by any amount and this made it
all the way to LowerToHW (and wasn't canonicalized away earlier).  FIRRTL
semantics interpret this case as a 1-bit zero.  Insert the 1-bit zero if
we ever see this.

Fixes #6652.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[ImportVerilog] Fix single unit preprocessor option (#6682)

Make `ImportVerilog` honor the `singleUnit` option by either adding all
source files to a single preprocessor (single unit), or creating a fresh
preprocessor for every single file (multiple units).

Fixes #6680.

[NFC][LowerIntrinsics] Add test & check for probes

[arcilator] Remove PrintStateInfo pass (#6529)

* PrintStateInfo refactoring

* remove printinfopass

* fix failing tests

* extract arcilator logic to tools lib

* add CIRCTArc dependency to Arcilator tool

* match argument names

* make state collection non-recursive

* move modelinfo to the arc dialect

* fix formatting

* address comments

* fix formatting

* revert llvm bump

* update old variable name

* rename file + lib

* fix formatting

LLVM bump (#6662)

Co-authored-by: Mike Urbach <mikeurbach@gmail.com>

Make circt-verilog available to integration tests. (#6685)

[LowerClasses] Lower classes that instantiate properties. (#6688)

The previous logic would create classes for any FIRRTL modules that
had property ports. However, if some intermediate modules had property
ports, but their parents did not, we would not convert their parents,
which would break pass invariants and lead to failed assertions.

This adds logic to also convert any FIRRTL modules that instantiate
FIRRTL modules that will be converted. This upholds the pass
invariants, and makes it robust in case property ports are used in
part of the hierarchy but not exported from the top-level module of
the circuit.

[NFC][LowerIntrinsics] Allow unknown intrinsics to be bypassed

[FIRRTL] Add some FileCheck directives to an existing test, NFC.

This test was added with a small patch recently, but the test didn't
really check anything beyond that the pass doesn't crash. This adds
some CHECK directives to ensure the intended lowering actually
happened.

[FIRRTL] Support probes in LowerLayers (#6554)

Updates to LowerLayers to support driving layer-associated probes.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Co-authored-by: Robert Young <rwy0717@gmail.com>

[CI] Bump the image versions on CI and integration tests

To accomodate upcoming slang 4 builds. Changes the version of clang in
the integration tests to 16.

[ExportVerilog] Fix crash on `sv.reg` with initial value (#6689)

* Change isMovableDeclaration to allow operations to be lifted if all their operands are constant expressions.
* Adapt lowerUsersToTemporaryWire to handle InOut values.

[CAPI] Try again to fix dependency leading to Python wheel failures.

The previous fix in https://github.com/llvm/circt/pull/6572 did not
get to the root of the issue. In fact, it didn't even work around the
issue correctly, because the added DEPENDS was on the wrong
target. This attempts to remedy that by changing the DEPENDS to the
actual target that was missing before.

The real issue to this problem is in the CFToHandshake.h header, and
tracked here: https://github.com/llvm/circt/issues/6693.

[HW] Encode the option group name in instance choice ops (#6645)

Also renamed `targetNames` to `caseNames` to better match the FIRRTL-level terminology

[HW] Remove ExternModKind in favor of static type reflection

[HW] HWModule: store input port locations only on block args

On modules we keep an array attribute `port_locs` for port location
information.  On regular HWModules specifically, which have a body
block, we copy the input port location to the entry block arguments, for
error messages and such. This is violating the "single source of truth"
principle for input port location info, so we added a verifier that
ensured the block argument locations were always kept in sync with the
port location attributes. This lead to another problem: if you print our
IR in the generalized format and try to read it back in, by default with
`-mlir-print-debug-info` printing turned off, it will end up throwing
away the location on the blocks. When the IR is read back in, the block
arguments will have new location information corresponding to the
intermediate file, while the location array maintains the old locations.
This trips the verifier since the port attribute location will be out of
sync with the block argument location.

To fix this problem, this changes the HWModule op to record input port
locations only on the blocks, and to store the output port locations in
a new `result_locs` array.  The other module types remain the same.
Now, when printing in the generic format, input ports will lose their
original location information, but the result locations will remain the
same.  If printing with debug information, then both will retain their
original locations.

As an example:
```mlir
// locs.mlir
hw.module @B(in %a: i1, out nameOfPortInSV: i1) {
  hw.output %a: i1
}
```

Printing without the debug information with`circt-opt
-mlir-print-op-generic locs.mlir | ./bin/circt-opt
--mlir-print-debuginfo` gives:
```mlir
module {
  hw.module @B(in %a : i1 loc(#loc2), out nameOfPortInSV : i1 loc(#loc3)) {
    hw.output %a : i1 loc(#loc4)
  } loc(#loc1)
} loc(#loc)
```

Printing with the debug information with `circt-opt
-mlir-print-op-generic --mlir-print-debuginfo locs.mlir |
./bin/circt-opt --mlir-print-debuginfo` gives:
```mlir
module {
  hw.module @B(in %a : i1 loc(#loc2), out nameOfPortInSV : i1 loc(#loc3)) {
    hw.output %a : i1 loc(#loc4)
  } loc(#loc1)
} loc(#loc)
```

[FIRRTL] Add parser version APIs that accept an SMLoc, NFC. (#6692)

These currently default to calling emitError(), which uses the current
token's location. In some cases, the current token may have been
consumed by the time we wish to check versions and potentially emit
errors. This allows users to supply an SMLoc if necessary, and
otherwise defaults to the previous behavior.

None of our current parsing actually consumes a token before checking
versions, but I would like to add a version check to an existing
parser that will come after it has consumed the token.

[FIRRTL] Provide a way to override inferReturnTypes in FIRRTLExprOp. (#6697)

This is currently always defining the inferReturnTypes method from
InferTypeOpInterface on all FIRRTLExprOps in ODS. However, some
simpler operations might want implement this method using the
SameOperandsAndResultTypes trait, for example.

This factors out that declaration into a TableGen variable that
subclasses can override as neeeded. This would be used, for example,
in https://github.com/llvm/circt/pull/6691/files.

[FIRRTL] Add integer addition property op. (#6691)

This op adds two FIntegerType operands to produce an FIntegerType
result. This defines the usual fold and result type inference hooks
required on BinaryPrimOp, but doesn't add any folders yet.

[OM] Add rationale for expressions. (#6702)

This adds some basic rationale for why it is valuable to be able to
represent computation in the OM dialect, with a couple examples
including arithmetic and container constructions.

[OM] Add OpInterface for IntegerBinaryArithmeticOp. (#6703)

As we start adding integer binary arithmetic expressions, this will
allow us to abstract over these ops in the evaluator. The interfaces
defines accessors for each operand and the result, which can be
trivially implemented by a shared helper in ODS. The interface also
defines one method to be used in the evaluator, which is to actually
evaluate the integer arithmetic operation. APSInts for each operand
are passed to the interface method, and an APSInt representing the
result or a failure must be returned.

[PyCDE] Refactor Input/Output ports to extend property (#6700)

Specify fget in the port class itself to that users still have access to the port's properties through the class variable instead of replacing the port itself. As a result, we only have to store a list of ports.

[FIRRTL] Add integer addition parser support. (#6701)

This adds parser support for IntegerAddOp by defining the primitive
expression keyword. This is only supported in FIRRTL version 4.0.0 and
up.

[NFC, HW] HWInstanceLike doesn't implement PortList.  Much of PortList would require instances to get the moduletype for what they are instantiating which plays poorly with hw.module passes (pass pipeline parallelization).

[CMake] Make ImportVerilog compile-time depend on slang (#6707)

ImportVerilog.cpp depends on header files which are generated during the build of the slang_slang target. Not having this dependeny in CMake can cause clean builds to fail.

[FIRRTL][Lower-Layers] do not capture uses multiple times (#6699)

Fixes: #6694

[FIRRTL] Change min width of shr for UInt to 0 (#6698)

Major change in FIRRTL 4.0.0.

---------

Co-authored-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Co-authored-by: Andrew Young <youngar17@gmail.com>

[OM] Update IntegerBinaryArithmeticInterface to use const refs.

We don't need to copy APSInts for the arguments in this interface
method, and in fact clang-tidy will complain unless implementors
accept const references. This changes the method signature to accept
two const references.

[FIRRTL] Use untyped propassign source accessor in LowerClasses. (#6690)

When lowering object instantiations, we need to get the source from a
propassign to pass that Value to the newly created OM dialect object
op. However, in some cases, like when passing references to objects
down the hierarchy, we have already converted the source Value to an
OM dialect object op. This means the typed getSrc accessor will fail,
as it expects the source Value to be a FIRRTLType.

The IR is in the desired state at this point, so instead we can simply
use the untyped getSrcMutable accessor to get the corresponding
OpOperand, and get the Value from there. This avoids errors when the
pass has knowingly put the IR into a state that won't pass the
verifier.

[OM] Add integer addition op. (#6704)

This op adds two OMIntegerType operands to produce an OMIntegerType
result. This defines the evaluateIntegerOperation interface method in
terms of APSInt +.

[HW] Fix several bugs related to input and output only updtes to modules.

Also mark modifyModulePorts as deprecated as it is input/output relative.

See #6706

[NFC] Expand a comment

[HW] Fix more bugs related to input/output specific functions.

Fix more hold-overs from when input and output were split.

See #6707

[NFC] Sort tools alphabetically

Hack: Run CI on GitHub runners

WIP: [ImportVerilog] Slang frontend integration

Co-authored-by: Fabian Schuiki <fabian@schuiki.ch>
Co-authored-by: ShiZuoye <albertethon@163.com>
Co-authored-by: hunterzju <hunter_ht@zju.edu.cn>
Co-authored-by: 孙海龙 <hailong.sun@terapines.com>

* [ImportVerilog] This PR is aimed at showing the features that Moore supports now. (#26)

* WIP: [ImportVerilog] Slang frontend integration

* [ImportVerilog] Add some new types which are equivalent to PredefinedIntegerType in slang::ast.

* [ImportVerilog] Add some new test cases to detect.

* [ImportVerilog] Add support for the RealType that is equivalent to FloatingType from slang::ast namespace.

* [ImportVerilog] Add some new test cases for RealType.

* [ImportVerilog] Add support for the moore::UnpackedUnsizedDim and moore::UnpackedRangeDim

* [ImportVerilog] Add some new test cases and adjust the order of CHECK-LABEL in this file.

* Add support for the moore::UnpackedAssocDim and moore::UnpackedQueueDim.

* Add some new test cases for UnpackedAssocDim and UnpackedQueueDim.

* Adjust the order of test cases to pass FileCheck correctly.

* [ImportVerilog] Add support for the NetType.

* [ImportVerilog] Add new test cases for the NetType.

* [ImportVerilog] Add support for the moore::EnumType, but there are no related test cases.

* [ImportVerilog] Some unnecessary semicolons were removed.

* [ImportVerilog] The handling of some IntTypes was deleted due to an earlier error and is now restored.

* [ImportVerilog] Able to handle 32-bit assignment Op.

* [ImportVerilog] Able to handle 32-bit assignment Op and handle single blocking assignment in always_comb block.

* [ImportVerilog] About the definition of Moore::AlwaysCombOp.

* [ImportVerilog] WIP:Can't handle cases where the right side of an AssignOp is a variable.

* [ImportVerilog] Add support for the moore::AlwaysCombOp and a new file called Statement.cpp is created, which is used for slang statement conversion.

* [ImportVerilog] Create a new file called Statement.cpp which is related to Slang statement conversion.

* [ImportVerilog] Change break in the case statement to call mlir::emitError().

* [ImportVerilog] There are tweaks to the framework for handling expressions. Andadd new supported for the handling varibles and nets.

* [ImportVerilog] Add a new file to handle expression specially.

* [ImportVerilog] Add a new line at the end of Expression.cpp.

* [ImportVerilog] Add a new line at the end of Expression.cpp.

* [ImportVerilog] Tweak a little detail.

* [ImportVerilog] Support continuous assignment, but rhs can not a variable.

* [ImportVerilog] Add support for the moore::IfOp.

* [ImportVerilog] Add two new files for slang statements and expressions conversion.  And now can support conversion of the alwaysCombOp, initialOp, and ifOp.

* Update CMakeLists.txt

* Update MooreOps.cpp

* Update Structure.cpp

* Update Structure.cpp

* [ImportVerilog] A lot of things, review the details at the framework branch.

* [ImportVerilog] Add EqualityOp to handle four equal operator.

* [ImportVerilog] Implement the body of the visitBinaryOp func for choosing which op will be used.

* [ImportVerilog] Implement the body of the visitSignalEvent func.

* [ImportVerilog] Add moore::IfOp and modify the type of AssignOp.

* [ImportVerilog] Support the relational and shift operators and introduce the scf::IfOp.

* [ImportVerilog] Add the moore::LogicalOp to handle four logical operators.

* [ImportVerilog] About the unaryOp, binaryOp, netOp and conversionOp. And add the test.sv file for testing existing feature.

* [ImportVerilog] Support AddOp, MulOp and improved assignment Op (#27)

* [ImportVerilog] add PCassignOp and fix test.sv

 Create pcassign op when expr is Procedural continuals
 assignment.
 Tweak `test.sv` to pass the FileCheck.
 Clean up unnecessary functions.

* [ImportVerilog] Support AddOp and MulOp

AddOp and MulOp support variadic operands.
The element of operands should be same.

WIP: Add procedure tests, add missing bail-on-error

WIP: Remove lvalue/rvalue type distinction

WIP: Clean up ops and make tests more focused

WIP: Minor restructuring

WIP: Improve cast op and add more tests

WIP: Use visitor to handle expressions

WIP: Remove unneeded conversion ops

WIP: Make ConstantOp more strict, add verifiers and builders

WIP: Refactor and add missing unary operators

WIP: Add more dedicated binary ops, add wildcard eq/neq, refactor

Squash: Drop MIR statements file

Squash: Move concat op

Squash: Update logical and, or, implication, equivalence

Squash: Rework shift operations

Squash: Remove MIR expressions file

Fix building problems after merging

[MooreToCore] Add support for the conversion of InstanceOp

1. Add support for the conversion of InstanceOp. Now, the mooreToCore pass support converting moore::InstanceOp to hw::InstanceOp.
2. Remove original logic of updating and removing port operations, it had been moved into rewriting pattern in the first procedure.
3. Fix materialization problems around conversion of several moore operations such as instanceOp, portOp ...
Addition: due to the fixing of materialization function, conversion of net / variable operation encounters a problem and it will be fixed in next commit.
Author: BuyunXu

[MooreToCore] Add calling function test

[MooreToCore][NFC] Remove an unused variable in InstanceOpConv

[MooreToCore] Fix the conversion problem of net & variable operation

[MooreToCore] Add a new test for net operation

[ImportVerilog] Support RangeSelect & ElementSelect features

Support translation the rangeselect and elementselect feature in Systemverilog. Now you can translate relating logic via --import-verilog
Author: hailongSun2000

[MooreToCore][NFC] Fix minor text in the test file

[NFC][SV] Fix whitespace

[NFC][HW] Pull the HW enums into a separate tablegen

MLIR fails to separate enums by dialect, thus including a `.td` file which defines in another results in duplicate enum declarations.
To address the problem, since enum attributes refer to the underlying enum via a string, the enums can be pulled into a standalone file.

[NFC][HW] Make references to hw::InstanceOp fully qualified

[FIRRTL] Fix zero-width enum parsing

Fix a bug in zero-width enum parsing that arose because the location was
not set.  This would cause a crash when creating a constant.

Fixes #6713.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] Use const in paseEnumExpr

Change the 'parseEnumExpr' to create a const zero-width zero and not a
non-const one.  This aligns this with how it creates non zero-width
constants as is shown in the test.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] Remove duplicate locationProcessor.setLoc

Remove a duplicate setting of the location when parsing enum expressions.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[FIRRTL] "groups_" -> "layers_" for FIRRTL 4.0.0

Change files generated by the LowerLayers pass to use a "layers_" prefix
and not a "groups_" prefix.  This was changed in FIRRTL 4.0.0.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[LLVM] Bump (#6718)

[FIRRTL] Update HierPathOps in LowerLayers

Fix a bug in the LowerLayers pass where operations with inner symbols
inside layerblocks would not have their HierPathOp users updated when
these operations were moved into new modules.  Fix this by recording a
mapping of modifications to InnerRefAttrs and then applying these to
HierPathOps after all modules are processed.

Fixes #6717.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

[ESI][Runtime] Use `std::future` in channel reads and func calls (#6723)

[FIRRTL] Add integer addition conversion to LowerClasses. (#6710)

This is a straightforward 1:1 conversion from FIRRTL IntegerAddOp to
OM IntegerAddOp, using the converted operand types from the
OpAdaptor.

[OM] Support integer binary arithmetic in the Evaluator. (#6711)

This adds support for the Evaluator to evaluate integer binary
arithmetic. This is defined in terms of the IntegerBinaryArithmeticOp
interface, so the Evaluator can generically handle all ops that
implement this interface in terms of the interface methods.

At a high level, getOrCreateValue will create a partially evaluated
value for the result of this op. Once the operands are fully
evaluated, they are passed to the evaluateIntegerOperation interface
method, and if successful, the result is used for the evaluator
value.

Because these are the first kind of operation using AttributeValues
that doesn't necessarily make the Attribute immediately available, the
AttributeValue has been extended to support construction in a
partially evaluated state to be filled in later.

The tests show example of straightforward evaluation where the
operands are immediately available from constants, where the operands
are references to object fields, and where the operands are references
to object fields that are initially not evaluated.

This handles many cases, but there is currently no detection for
dataflow cycles through an integer binary arithmetic operation. The
evaluator would need significant changes in the current setup to
detect this on the fly, so currently assumes cycles are prevented
earlier in the pipeline.

[FIRRTL] Add integer multiplication property op.

This op multiplies two FIntegerType operands to produce an
FIntegerType result. This defines the usual fold and assembly format
name hooks required on BinaryPrimOp, but doesn't add any folders yet.

[FIRRTL] Add integer multiplication parser support.

This adds parser support for IntegerMulOp by defining the primitive
expression keyword. This is only supported in FIRRTL version 4.0.0 and
up.

[OM] Add integer multiplication op.

This op multiplies two OMIntegerType operands to produce an
OMIntegerType result. This defines the evaluateIntegerOperation
interface method in terms of APSInt *.

[FIRRTL] Add integer multiplication conversion to LowerClasses.

This is a straightforward 1:1 conversion from FIRRTL IntegerMulOp to
OM IntegerMulOp, using the converted operands types from the
OpAdaptor.

[FIRRTL] Add integer shift right property op.

This op shifts an FIntegerType lhs operand to the right by an
FIntegerType rhs operand. This defines the usal fold and assembly
format name hooks required by BinaryPrimOp, but doesn't add any
folders yet.

[FIRRTL] Add integer shift right parser support.

This adds parser support for IntegerShrOp by defining the primitive
expression keyword. This is only supported in FIRRTL version 4.0.0 and
up.

[OM] Add integer shift right op.

This op shifts its lhs OMIntegerType operand right by the rhs
OMIntegerType operand to produce an OMIntegerType result. This defines
the evaluateIntegerOperation interface method in terms of APSInt >>.

If the shift amount is negative, an error is raised according to the
definition of the operation semantics.

Since the implementation simply uses the getExtValue of the shift
amount, this also raises an error if the shift amount does not fit
into an int64_t.

[FIRRTL] Add integer shift right conversion to LowerClasses.

This is a straightforward 1:1 conversion from FIRRTL IntegerShrOp to
OM IntegerShrOp, using the converted operands types from the
OpAdaptor.

[PyCDE] Misc cleanups

Cleanups which have been accumulating and are mostly unrelated to
a new feature.

[PyCDE] Satisfy yapf

[FIRTOOL] More sane chisel interface directory handling (#6687)

Deal with a few more situations when outputting chisel interfaces.

[FIRRTL] Prevent division by zero in CreateSiFiveMetadata (#6726)

Fixes a crash when compiling a Chisel read only memory with a LoadMemoryFromFile annotation using the --repl-seq-mem option.

[ImportVerilog] Add translation, run Slang compilation (#6708)

Add an `--import-veril…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant