Skip to content

[FIRRTL] Implement intrinsic module pla #6381

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SpriteOvO
Copy link
Member

Currently this PR just shows what it looks like, it is not tested and may not work as expected.

@darthscsi
Copy link
Contributor

I'm assuming this will have a more descriptive name if it is proposed.

@darthscsi
Copy link
Contributor

It's not obvious this should be an intrinsic rather than a chisel library. Intrinsics are generally reserved for things which require either implementation knowledge (e.g. sizeof needs to reason about the post-inferred type of it's argument), or need to produce specific non-standard output which is hard to constrain existing constructs to (e.g. muxcell must map to mux cell libraries).

@sequencer
Copy link
Contributor

It's not obvious this should be an intrinsic rather than a chisel library. Intrinsics are generally reserved for things which require either implementation knowledge (e.g. sizeof needs to reason about the post-inferred type of it's argument), or need to produce specific non-standard output which is hard to constrain existing constructs to (e.g. muxcell must map to mux cell libraries).

This was discussed in the Chisel dev meeting, sorry for not updating the detail documentation here.

The reason implementing PLA is that Chisel wants to migrate decoding logic from Scala to CIRCT in the near feature, since the decoder circuit backend in Chisel is PLA, the first step of migration is implementing a PLA intmodule which allows the Decoder intmodule(in the following PR) being able to be lowered to PLA intmodule in the CIRCT pass. Here is the migrating step in my mind:

  • Implement PLA intmodule in CIRCT;
  • Add this intmodule to Chisel to experimentally replace original Chisel generator;
  • Implement Decoder intmodule;
  • Lower Decoder to PLA via espresso or QMC to PLA intmodule;

After finishing this, we can provide other backend, e.g. casez backend for Decoder to provide a better Verilog or QoR in commercial synthesis tools, e.g. SNPS DC already implemented multi-stage decoding algorithm which provide better PPA in high performance designs. But it only works a specific Verilog coding style.

@darthscsi
Copy link
Contributor

The goal is not to implement a bunch of piece-wise ops matching various parts of the chisel library, the goal is to provide a set of building blocks to construct interesting things. The basic operation needed seems to be match on a bitpat. From this you can build all kinds of decoders. You can also build plenty of optimizers since walking a mux structure of matches is straight-forward (only issue may be that this encodes priority). A generic wild-card match operation would also help LTL expressions I suspect.

You also want to design the IR such that you get good output no mater the input. The more ways of expressing equivalent functionality, the more the output will depend on accidents of the input. Code generation falls in to a very EDA-style pattern-match heavy approach which provides a very inconsistent user experience. It is valid to capture higher-level constructs if reconstructing them is hard and you are optimizing on properties of those which are hard to reconstruct once lowered, but this is a tradeoff (against consistent output quality).

@fabianschuiki should be involved (due to LTL and verif constructs) to help make sure we don't wind up with redundant representations.

@fabianschuiki
Copy link
Contributor

fabianschuiki commented Nov 10, 2023

Hey @SpriteOvO, @sequencer! Thanks for starting an effort to move the synthesis pieces from Chisel into CIRCT. That's very exciting 🥳

It would be great if we could somehow get the Espresso/QMC interaction represented in CIRCT through some dedicated operation that encapsulates a region of comb ops that should be minimized. The fact that some of these minimizers use the Disjunctive Normal Form (DNF) to represent inputs and outputs -- basically a truth table of the logic function -- sounds like a quirk of the tool, and not necessarily something we'd want to represent in the IR. What I'm getting at is: could we make the conversion to and from DNF part of the callout to Espresso, and not have it represented directly in the IR as a PLA intrinsic or op?

There is a lot of value in having a good representation for lookup tables and these PLA-style DNFs in CIRCT. Other dialects and tools likely want to take advantage of those as well! For example, the Arc dialect has an arc.lut operation that represents lookup tables, and an accompanying transformation pass that performs LUT conversion in a way that's useful for Arc. I'm pretty sure that that LUT operation would be something we'd want to be able to feed through Espresso. And the result of the minimization may be best represented as such a LUT operation as well.

My suggestion would be to not have a PLA FIRRTL intrinsic, because all the intrinsic does right now is get converted to the Disjunctive Normal Form in LowerIntrinsics by creating the corresponding two-level AND/OR logic. This sounds like something that Chisel itself is very capable of doing. Instead, could we start out by defining some CIRCT operation that can somehow represent the intent of minimizing the logic of a sequence of operations? Chisel could then emit arbitrary logic, and the CIRCT pass would, when calling out to Espresso, convert everything into DNF or truth table form, do the minimization, and then convert back to some useful CIRCT representation.

Just some random thoughts. It's very exciting that we start pushing into the EDA/synthesis/optimization territory! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants