llvm / llvm/circt

[Calyx] Consider making RegisterOp and MemoryOp derive CalyxLibraryOp in ODS.

Open
#2,957 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Calyx enhancement good first issue
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

These two ops currently directly derive from CalyxPrimitiveOp, but they're not much more special than the other library ops. They're defined in the Calyx core primitive library here: https://github.com/cucapra/calyx/blob/cd6e4279a18e8f19ffba124a366defd0996548e9/primitives/core.futil#L27-L100.

One difference is I took the liberty of defining MemoryOp with a variadic number of ports that allow one op to represent any dimension of memory. If we implement this issue, that would go away, and there would be ops that map 1:1 to the Calyx primitives. Honestly that might be preferable, since a slight difference seems worse to me compared to total alignment.

At the least, I think we can make RegisterOp match its core primitive directly, by deriving CalyxLibraryOp. This should hopefully mean sharing more code now and in the long term.

Eventually, we plan to enhance the Calyx dialect to support defining primitives (https://github.com/llvm/circt/issues/2923). This is orthogonal to that, but it seems good to think about and potentially address before any enhancements.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Compare the ODS definitions of RegisterOp and MemoryOp with the Calyx core primitive library in primitives/core.futil, especially lines 27-100. Determine whether MemoryOp's variadic ports should be replaced and whether both operations can derive from CalyxLibraryOp. Done means the ODS operations align with the corresponding Calyx primitives and the existing behavior remains covered by the project’s tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.