[ImportVerilog] Add queue insert/delete ops by Lauriethefish · Pull Request #9632 · llvm/circt

Lauriethefish · 2026-02-07T17:27:53Z

Requires adding support for arity-3 system calls (Insert takes two parameters plus a queue), which has been done following the existing convention. Other than that, the work is roughly the same as in #9537.

The Delete method has been split into two Moore ops, one for each of its overloads:

If an index is supplied, QueueDeleteOp removes the element at that index.
If no index is supplied, QueueClearOp clears the entire queue.

Insert is implemented as QueueInsertOp.

If @Scheremo or @TaoBi22 could review when available 😃

Requires adding support for arity-3 system calls. The `Delete` method has been split into two moore ops, one for each of its overloads: - If an index is supplied, `QueueDeleteOp` removes the element at that index. - If no index is supplied, `QueueClearOp` clears the entire queue.

TaoBi22

This looks great! A couple of comments but they're both super minor

include/circt/Dialect/Moore/MooreOps.td

TaoBi22 · 2026-02-09T16:59:06Z

include/circt/Dialect/Moore/MooreOps.td

+      See IEEE 1800-2023 § 7.10.2.2 "Insert()"
+  }];
+  let arguments = (ins QueueType:$queue, TwoValuedI32:$index, UnpackedType:$item);
+  let results = (outs VoidType:$out);


I think we can probably just not declare any results on these ops (I see @Scheremo used this VoidType result style on some existing queue ops though, @Scheremo is there a specific reason for that?)

Perhaps: It makes for a more consistent style in convertSystemCallArity*. If the return type is a VoidType then it is possible to return the result of QueueDeleteOp::create directly instead of needing to have a separate line return Value();

I'll make a commit locally that removes all the instances of VoidType, then depending on the outcome of this conversation I'll push it to the branch.

This turns out to be quite an annoying one to fix: convertSystemCall emits the error unsupported system call if the result returned is a null value, and VoidType conveniently gets around this. Assuming we want to avoid a significant refactor, it might be easiest to keep VoidType.

I agree with @TaoBi22: it would be great to have these ops not return a result 👍. Better to sort the weird SystemVerilog expression/statement stuff out during the AST-to-IR conversion. The result-less ops should work nicely from the system call conversion in Statements.cpp (see other comment) 😃

I publicly agree with both @TaoBi22 and @fabianschuiki, I think we can drop the VoidType return value on all queue ops 🤔

fabianschuiki

Hey @Lauriethefish, thanks a lot for working on this. This is a really cool addition to the Verilog frontend 🥳. Just a minor comment about op name and result type. Otherwise this looks good to me!

fabianschuiki · 2026-02-09T19:00:01Z

lib/Conversion/ImportVerilog/Expressions.cpp

+          .Case("delete",
+                [&]() -> Value {
+                  if (isa<moore::QueueType>(value.getType())) {
+                    return moore::QueueClearOp::create(builder, loc, value);
+                  }
+                  return {};
+                })


Out of curiosity: do you know if you can use a q.delete(...) in an expression? If I recall correctly from SV, they don't return a result, or some weird notion of a void type in SV which you can never use in an expression.

You migth be able to add support for this result-less queue ops to the Statements.cpp file instead: https://github.com/llvm/circt/blob/main/lib/Conversion/ImportVerilog/Statements.cpp#L855-L861. That way you don't have to deal with result types 😃

fabianschuiki · 2026-02-09T19:00:37Z

test/Conversion/ImportVerilog/basic.sv

+       q.delete(1);
+       q.delete();
+       q.insert(0, 1);


Very cool 😎

fabianschuiki · 2026-02-09T19:02:55Z

include/circt/Dialect/Moore/MooreOps.td

+      See IEEE 1800-2023 § 7.10.2.2 "Insert()"
+  }];
+  let arguments = (ins QueueType:$queue, TwoValuedI32:$index, UnpackedType:$item);
+  let results = (outs VoidType:$out);


I agree with @TaoBi22: it would be great to have these ops not return a result 👍. Better to sort the weird SystemVerilog expression/statement stuff out during the AST-to-IR conversion. The result-less ops should work nicely from the system call conversion in Statements.cpp (see other comment) 😃

fabianschuiki · 2026-02-09T19:16:36Z

include/circt/Dialect/Moore/MooreOps.td

  }];
 }

+def QueueInsertOp : MooreOp<"insert", [


Do you mind prefixing these ops with queue.? I think we'll have more than one insert op, one for each container type. Something like this:

Suggested change

def QueueInsertOp : MooreOp<"insert", [

def QueueInsertOp : MooreOp<"queue.insert", [

(We probably should tweak push_front and friends in the future, too.)

fabianschuiki

Hey @Lauriethefish, thanks a lot for working on this. This is a really cool addition to the Verilog frontend 🥳. Just a minor comment about op name and result type. Otherwise this looks good to me!

Scheremo

Just realised I never submitted this review!
Mostly LGTM. I would agree with @TaoBi22 and @fabianschuiki on outstanding action items.

Scheremo · 2026-02-09T19:26:20Z

include/circt/Dialect/Moore/MooreOps.td

+      See IEEE 1800-2023 § 7.10.2.2 "Insert()"
+  }];
+  let arguments = (ins QueueType:$queue, TwoValuedI32:$index, UnpackedType:$item);
+  let results = (outs VoidType:$out);


I publicly agree with both @TaoBi22 and @fabianschuiki, I think we can drop the VoidType return value on all queue ops 🤔

Lauriethefish force-pushed the queue-ins-del branch from ef61623 to ca2a66e Compare February 9, 2026 15:24

TaoBi22 approved these changes Feb 9, 2026

View reviewed changes

include/circt/Dialect/Moore/MooreOps.td Outdated Show resolved Hide resolved

include/circt/Dialect/Moore/MooreOps.td Outdated Show resolved Hide resolved

TaoBi22 reviewed Feb 9, 2026

View reviewed changes

Lauriethefish added 3 commits February 9, 2026 17:31

Remove unnecessary index keyword

da8af19

the -> a for consistency

13808e0

Remove index from tests

14cf008

fabianschuiki approved these changes Feb 9, 2026

View reviewed changes

Scheremo reviewed Feb 9, 2026

View reviewed changes

	def QueueInsertOp : MooreOp<"insert", [
	def QueueInsertOp : MooreOp<"queue.insert", [

Conversation

Lauriethefish commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TaoBi22 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lauriethefish Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianschuiki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianschuiki left a comment

Choose a reason for hiding this comment

Uh oh!

Scheremo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Lauriethefish commented Feb 7, 2026 •

edited

Loading

Lauriethefish Feb 9, 2026 •

edited

Loading