llvm / llvm/circt

[CIRCT-VERILOG][ARC] arcilator fails on valid SystemVerilog always_ff with async reset

Open
#10,186 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

## [CIRCT-VERILOG][ARC] arcilator fails on valid SystemVerilog always_ff with async reset

**Tool**: CIRCT (circt-verilog + arcilator)
**Version**: 1.144.0
**Severity**: Compilation Error

### Description

A valid SystemVerilog `always_ff` block with an asynchronous reset (`or posedge rst`) triggers a compilation error in the CIRCT arcilator tool. The `StripSV` pass throws "only synchronous resets are currently supported" even though the `seq.firreg` operation is explicitly designed to support async resets via the `isAsync` attribute. This is a regression where CIRCT can parse and represent async resets but fails to process them.

### Minimal Reproducible Example

```systemverilog
module m(input clk, input rst);
logic r;
always_ff @(posedge clk or posedge rst) if (rst) r <= 0; else r <= 0;
endmodule
```

### Reproduction Steps

```bash
circt-verilog --ir-hw minimal_testcase.sv -o /tmp/circt_out.mlir
arcilator /tmp/circt_out.mlir -o /tmp/arcilator.ll
```

### Actual Output

```
/tmp/circt_out.mlir:5:10: error: 'seq.firreg' op only synchronous resets are currently supported
%r = seq.firreg %false clock %0 reset async %rst, %false : i1
^
/tmp/circt_out.mlir:5:10: note: see current operation: %2 = "seq.firreg"(%0, %1, %arg1, %0) <{isAsync, name = "r"}> : (i1, !seq.clock, i1, i1) -> i1
```

### Expected Output

The code should compile successfully. The `seq.firreg` operation correctly captures the async reset semantics (as shown by `reset async %rst` in the MLIR), but the `StripSV` pass fails to process it instead of properly lowering it to arc-level constructs.

### Cross-Tool Comparison

| Tool | Version | Result |
|------|---------|--------|
| CIRCT (circt-verilog + arcilator) | 5dc62fe (LLVM 23.0.0git) | FAIL - Compilation error in StripSV pass |
| Verilator | 5.046 2026-02-28 | PASS - Compiles successfully |
| Icarus Verilog | 13.0 (stable) | PASS - Compiles and simulates successfully |

### Root Cause (Preliminary)

The `StripSV` pass in `lib/Dialect/Arc/Transforms/StripSV.cpp:140-142` emits an error when it encounters `seq.firreg` with `isAsync=true`, instead of properly lowering async reset registers. The `seq.firreg` operation CAN represent async resets (defined in `SeqOps.td:238`, built in `SeqOps.cpp:419-430`, parsed in `SeqOps.cpp:456-467`), and `LowerToHW.cpp:3827-3831` correctly sets `isAsync=true` for `AsyncResetType` — but the `StripSV` pass does not handle it. Workaround: `--async-resets-as-sync` flag (changes semantics).

### Standard Compliance Assessment

**Classification**: tool_bug

**IEEE References**: IEEE 1800-2017 Section 9.2.2.4 (page 209) — "Sequential logic always_ff procedure" explicitly allows asynchronous resets in the event control. Syntax `always_ff @(posedge clock or posedge reset)` for async reset is valid standard SystemVerilog.

**Analysis**:
1. The SV code `always_ff @(posedge clk or posedge rst)` with `posedge rst` in the sensitivity list is **valid IEEE 1800**.
2. The `seq.firreg` operation **CAN represent** async resets — all CIRCT internal components (operation definition, builder, parser, HW lowering) correctly support `isAsync`.
3. The error comes from `StripSV` failing to **process** async resets, not from a representation limitation.
4. Verilator and Icarus Verilog both successfully compile the same code.

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

Run the circt-verilog and arcilator reproduction commands, then inspect lib/Dialect/Arc/Transforms/StripSV.cpp around lines 140-142. Read the seq.firreg definitions in SeqOps.td and SeqOps.cpp and the async-reset handling in LowerToHW.cpp around lines 3827-3831. Done means the minimal always_ff example lowers and compiles without changing its asynchronous reset semantics.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.