[circt-verilog][arcilator] Inconsistent compilation behavior: direct array indexing in always_ff sensitivity list vs. intermediate wire

[m2kar]

[circt-verilog][arcilator] Inconsistent compilation behavior: direct array indexing in always_ff sensitivity list vs. intermediate wire

Summary

I encountered an inconsistent compilation behavior in the arcilator flow when using synchronous reset with SystemVerilog array indexing. When directly using array indices (clkin_data[0] and clkin_data[32]) in an always_ff block, the compilation fails with an llhd.constant_time error. However, when the same signals are first assigned to intermediate wires and then used in the always_ff block, the compilation succeeds. This suggests a potential issue in how circt-verilog handles array indexing in sensitivity lists.

Problem Description

The SystemVerilog design uses a 64-bit input clkin_data where:

• clkin_data[0] is used as the clock signal
• clkin_data[32] is used as the reset signal (active low)

Two semantically equivalent approaches to implement synchronous reset logic produce different compilation results:

Solution 1 - Direct array indexing (compilation fails❌):

always_ff @(posedge clkin_data[0])
  if (!clkin_data[32]) _00_ <= 6'h00;
  else _00_ <= in_data[7:2];

Solution 2 - Intermediate wires (compilation succeeds✅):

wire clkin_0 = clkin_data[0];
wire rst = clkin_data[32];
always_ff @(posedge clkin_0) begin
  if (!rst) _00_ <= 6'h00;
  else _00_ <= in_data[7:2];
end

Both solutions implement identical synchronous reset logic, but Solution 1 fails during the circt-verilog --ir-hw to arcilator conversion pipeline with an llhd.constant_time error. From a SystemVerilog semantics perspective, these two approaches should be equivalent. The compilation failure in Solution 1 appears to be a tool limitation rather than a semantic difference.

Reproduction

Note: The test cases described below (top1.sv and top2.sv) are simplified versions that isolate the core issue. The original complete test case was generated by Yosys and includes additional combinational logic (folding blocks). The full original test case is provided in the Original Complete Test Case section below.

Solution 1: Direct Array Indexing (Fails)

Create top1.sv:

module top_arc(clkin_data, in_data, out_data);
  reg [5:0] _00_;
  input [63:0] clkin_data;
  wire [63:0] clkin_data;
  input [191:0] in_data;
  wire [191:0] in_data;
  output [191:0] out_data;
  wire [191:0] out_data;

  always_ff @(posedge clkin_data[0])
    if (!clkin_data[32]) _00_ <= 6'h00;
    else _00_ <= in_data[7:2];
endmodule

Compile with:

circt-verilog --ir-hw top1.sv | arcilator --state-file=top1.json | opt -O3 --strip-debug -S | llc -O3 --filetype=obj -o top1.o

Result: ❌ Compilation fails with the following error:

<stdin>:5:10: error: failed to legalize operation 'llhd.constant_time' that was explicitly marked illegal
    %0 = llhd.constant_time <0ns, 0d, 1e>
         ^
<stdin>:5:10: note: see current operation: %2 = "llhd.constant_time"() <{value = #llhd.time<0ns, 0d, 1e>}> : () -> !llhd.time
<stdin>:1:1: error: conversion to arcs failed

Note: The error occurs when piping the output directly from circt-verilog to arcilator, so the error location shows <stdin> instead of a file name. The error happens during the arcilator conversion phase.

Solution 2: Intermediate Wires (Succeeds)

Create top2.sv:

module top_arc(clkin_data, in_data, out_data);
  reg [5:0] _00_;
  input [63:0] clkin_data;
  wire [63:0] clkin_data;
  input [191:0] in_data;
  wire [191:0] in_data;
  output [191:0] out_data;
  wire [191:0] out_data;

  wire clkin_0 = clkin_data[0];
  wire rst = clkin_data[32];
  always_ff @(posedge clkin_0) begin
    if (!rst) _00_ <= 6'h00;
    else _00_ <= in_data[7:2];
  end
endmodule

Compile with:

circt-verilog --ir-hw top2.sv | arcilator --state-file=top2.json | opt -O3 --strip-debug -S | llc -O3 --filetype=obj -o top2.o

Result: ✅ Compilation succeeds, generates top2.o and top2.json

Expected Result

Both Solution 1 and Solution 2 should compile successfully since they are semantically equivalent. The direct use of array indices in sensitivity lists should be supported, or at minimum, a clear error message should indicate that intermediate wires are required.

Analysis of Failure Cause

Based on the error message, the failure occurs during the conversion from LLHD to arcs:

1. Solution 1 generates llhd.constant_time operations when array indices are used directly in the sensitivity list
2. The ConvertToArcs pass marks llhd.constant_time as illegal and does not provide a legalization pattern
3. Solution 2 avoids this issue by using intermediate wires, which apparently results in a different LLHD representation that does not generate llhd.constant_time operations

The root cause appears to be in how circt-verilog handles array indexing in sensitivity lists. When array indices are used directly, it may be generating timing-related operations (llhd.constant_time) that are not supported by the arcilator pipeline. Using intermediate wires changes the IR representation in a way that avoids these problematic operations.

Questions

1. Why does direct array indexing in sensitivity lists generate llhd.constant_time operations?
2. Is this a known limitation, or should it be considered a bug?
3. Should circt-verilog be able to handle direct array indexing in sensitivity lists, or is using intermediate wires the recommended approach?
4. If intermediate wires are required, should the compiler provide a clearer error message or automatically insert them?

Environment

• OS: Linux 5.15.0-164-generic
• CIRCT: firtool-1.139.0 (or current development version)
• LLVM: 22.0.0git

Original Complete Test Case

Original Complete Test Case

The original test case was generated by Yosys and includes the problematic always_ff block along with additional combinational logic (folding blocks). The complete file is shown below:

/* Generated by Yosys 0.37+29 (git sha1 3c3788ee2, clang 18.1.3 -fPIC -Os) */

module top(clkin_data, in_data, out_data);
  reg [5:0] _00_;
  wire celloutsig_0_0z;
  reg [2:0] celloutsig_0_2z;
  wire [3:0] celloutsig_0_3z;
  wire celloutsig_0_4z;
  wire celloutsig_0_8z;
  wire celloutsig_0_9z;
  wire celloutsig_1_0z;
  wire celloutsig_1_10z;
  wire celloutsig_1_11z;
  wire [2:0] celloutsig_1_13z;
  wire celloutsig_1_14z;
  wire [2:0] celloutsig_1_15z;
  wire celloutsig_1_18z;
  wire [3:0] celloutsig_1_19z;
  wire celloutsig_1_1z;
  wire celloutsig_1_2z;
  wire celloutsig_1_4z;
  wire [26:0] celloutsig_1_5z;
  wire celloutsig_1_6z;
  wire [2:0] celloutsig_1_7z;
  wire [13:0] celloutsig_1_8z;
  wire celloutsig_1_9z;
  input [63:0] clkin_data;
  wire [63:0] clkin_data;
  input [191:0] in_data;
  wire [191:0] in_data;
  output [191:0] out_data;
  wire [191:0] out_data;
  assign celloutsig_1_9z = ~celloutsig_1_1z;
  assign celloutsig_1_2z = celloutsig_1_1z | celloutsig_1_0z;
  assign celloutsig_1_4z = celloutsig_1_0z | celloutsig_1_2z;
  

  // Original approach (async reset, not supported by Arcilator)
  // ❌ Compilation fails
  always_ff @(posedge clkin_data[0], negedge clkin_data[32])
    if (!clkin_data[32]) _00_ <= 6'h00;
    else _00_ <= in_data[7:2];
  /*
  Compile Error Log:

  top_arc.mlir:6:10: error: failed to legalize operation 'llhd.constant_time' that was explicitly marked illegal
    %0 = llhd.constant_time <0ns, 1d, 0e>
         ^
top_arc.mlir:6:10: note: see current operation: %3 = "llhd.constant_time"() <{value = #llhd.time<0ns, 1d, 0e>}> : () -> !llhd.time
top_arc.mlir:1:1: error: conversion to arcs failed
   */

  // Note: Arcilator is a cycle-accurate simulator that only supports synchronous reset
  // True async reset semantics (reset signal takes effect immediately on falling edge) cannot be achieved
  // However, we can approximate async reset behavior through the following approaches:
  // 1. Check reset signal on clock rising edge, clear immediately if reset is low
  // 2. Reset signal must be stable at clock rising edge (meet setup/hold time requirements)
  // 3. Reset response delay is at most one clock cycle (main difference between sync and async reset)

  // Solution 1 (synchronous reset, Arcilator compatible)
  // ❌ Compilation fails
  always_ff @(posedge clkin_data[0])
    // Check reset signal on clock rising edge
    if (!clkin_data[32]) _00_ <= 6'h00;
    else _00_ <= in_data[7:2];
   /*
  Compile Error Log:

  top_arc.mlir:6:10: error: failed to legalize operation 'llhd.constant_time' that was explicitly marked illegal
    %0 = llhd.constant_time <0ns, 1d, 0e>
         ^
top_arc.mlir:6:10: note: see current operation: %3 = "llhd.constant_time"() <{value = #llhd.time<0ns, 1d, 0e>}> : () -> !llhd.time
top_arc.mlir:1:1: error: conversion to arcs failed
   */
  

  // Solution 2 (synchronous reset, Arcilator compatible)
  // ✅ Compilation succeeds

  // Define additional clock and reset wires
  wire clkin_0 = clkin_data[0];
  wire rst = clkin_data[32];
  
  always_ff @(posedge clkin_0) begin
    // Check reset signal on clock rising edge
    if (!rst) _00_ <= 6'h00;
    else _00_ <= in_data[7:2];
  end



  assign celloutsig_0_3z = { celloutsig_0_0z, celloutsig_0_2z } & { celloutsig_0_2z[0], celloutsig_0_2z };
  assign celloutsig_1_1z = in_data[185:171] && { in_data[181:170], celloutsig_1_0z, celloutsig_1_0z, celloutsig_1_0z };
  assign celloutsig_0_0z = ! in_data[82:51];
  assign celloutsig_0_4z = ! { celloutsig_0_3z[3:2], celloutsig_0_0z, celloutsig_0_3z, celloutsig_0_0z, celloutsig_0_2z, _00_ };
  assign celloutsig_1_10z = celloutsig_1_5z[22:8] < celloutsig_1_5z[15:1];
  assign celloutsig_1_19z = { celloutsig_1_7z, celloutsig_1_18z } % { 1'h1, celloutsig_1_13z };
  assign celloutsig_1_7z = { celloutsig_1_2z, celloutsig_1_0z, celloutsig_1_2z } * in_data[172:170];
  assign celloutsig_1_13z = in_data[139] ? { celloutsig_1_8z[13:12], celloutsig_1_0z } : celloutsig_1_7z;
  assign celloutsig_0_9z = { in_data[78:61], celloutsig_0_0z, celloutsig_0_3z } !== { in_data[77:56], celloutsig_0_8z };
  assign celloutsig_1_6z = celloutsig_1_0z & celloutsig_1_5z[26];
  assign celloutsig_1_18z = | celloutsig_1_15z;
  assign celloutsig_1_14z = ^ { celloutsig_1_11z, celloutsig_1_0z, celloutsig_1_6z, celloutsig_1_6z };
  assign celloutsig_1_8z = { celloutsig_1_5z[18:8], celloutsig_1_2z, celloutsig_1_0z, celloutsig_1_6z } >>> { in_data[116:112], celloutsig_1_7z, celloutsig_1_0z, celloutsig_1_6z, celloutsig_1_0z, celloutsig_1_2z, celloutsig_1_1z, celloutsig_1_4z };
  assign celloutsig_1_15z = { celloutsig_1_14z, celloutsig_1_9z, celloutsig_1_11z } - { celloutsig_1_13z[1:0], celloutsig_1_0z };
  assign celloutsig_1_5z = { in_data[162:145], celloutsig_1_4z, celloutsig_1_0z, celloutsig_1_0z, celloutsig_1_2z, celloutsig_1_0z, celloutsig_1_0z, celloutsig_1_4z, celloutsig_1_2z, celloutsig_1_1z } ~^ in_data[134:108];
  assign celloutsig_0_8z = ~((celloutsig_0_0z & celloutsig_0_4z) | celloutsig_0_4z);
  assign celloutsig_1_0z = ~((in_data[134] & in_data[145]) | in_data[182]);
  always_latch
    if (!clkin_data[32]) celloutsig_0_2z = 3'h0;
    else if (!celloutsig_1_18z) celloutsig_0_2z = in_data[83:81];
  assign celloutsig_1_11z = ~((celloutsig_1_10z & celloutsig_1_9z) | (celloutsig_1_4z & celloutsig_1_6z));
  assign { out_data[128], out_data[99:96], out_data[32], out_data[0] } = { celloutsig_1_18z, celloutsig_1_19z, celloutsig_0_8z, celloutsig_0_9z };
endmodule

This complete test case demonstrates the same issue: when using direct array indexing (clkin_data[0] and clkin_data[32]) in the always_ff sensitivity list, the compilation fails. The additional combinational logic (folding blocks) does not affect the core issue but provides a more realistic test case scenario.

Notes

• The error occurs during the arcilator conversion phase when processing the LLHD IR generated by circt-verilog --ir-hw
• The same pattern (direct array indexing vs. intermediate wires) may affect other SystemVerilog constructs beyond always_ff blocks
• This issue is related to but distinct from issue [circt-verilog][arcilator] arcilator fails to lower llhd.constant_time generated from simple SV delay (#1) #9467 , which deals with #1 delays generating llhd.constant_time operations

[5iri]

From what I understand, ConvertToArcs doesn't support LLHD operations at all. LLHD should have been lowered to hw/comb/seq as that is what it accepts.

The ideal solution should definitely not be adding a intermediate wire, and compiles solution one as well.

This seems to be a legitimate bug to me.

[5iri]

ahh!

The frontend doesn't seem to accept clkin_data[0] as a clock and uses LLHD for always_ff

if it finds the clock, it lowers straight to seq.firreg and no LLHD shows up!

I think the best way to solve this is by making somehow finding the place where it differentiates between single bit and bit select and mix them together; somewhere in the WaitEventOpConversion class.

[fabianschuiki]

Thanks for finding and reporting this! I totally agree with your findings. I see two issues here that create this problem:

1. The LLHD lowering pipeline should accept clkin_data[0] as a clock signal and properly lower the always_ff to a seq.firreg. If this worked, your code would compile.
2. Arcilator should really support LLHD, which would also make your code work.

[5iri]

I somehow find Arcilator supporting LLHD to be too much troublesome, since it will increase maintenance and the tests that we would need and also will definitely increase compile time.

Rather I feel improving the lowering technique would make the most minimal change and will also fix this issue much better.

[fabianschuiki]

I agree, we should definitely fix the lowering. Users will expect the clock you wrote to just work and produce a register.

[5iri]

can you assign me this issue? I will work on this right away!

[fabianschuiki]

Done! Thanks a lot for taking a stab at this 👍

[fabianschuiki]

You should be able to run the small example:

module Foo(input logic [1:0] a, input logic [41:0] b, output logic [41:0] c);
  always_ff @(posedge a[0]) begin
    if (!a[1])
      c <= '0;
    else
      c <= b;
  end
endmodule

through circt-verilog --mlir-print-ir-before-all. That will print the IR before each lowering pass. You are probably interested in the IR just before the Deseq pass, which is where processes get converted to registers. The LLHD Mem2Reg pass may also do interesting parts of the lowering that could help or prevent the register conversion.

Once you have that IR before Deseq, you can copy-paste it into a new file and use circt-opt --llhd-deseq your-file.mlir to only run Deseq. That will allow you to work on the individual pass and very quickly test your changes. You can also enable debug prints for that pass, which can be very helpful when finding issues or developing the pass: circt-opt --llhd-deseq --debug-only=llhd-deseq.

[5iri]

ohh interesting! did not know about this!

[fabianschuiki]

My suspicion is that it's actually not a problem in Deseq, but the earlier Mem2Reg and HoistSignals passes might not have properly handled the a[0] access. If that's the case, Deseq will probably complain about llhd.prb ops still being in the process.

[m2kar]

Thank you all . I have tried #9481 to confirm that the issue has been resolved.