Skip to content

[FIRRTL] Improve Error Messages in InferDomains #10041

@seldridge

Description

@seldridge

At present, the error messages that are produced by FIRRTL's InferDomains pass
are somewhat difficult for a user (me!) to process.

Consider the following example. This is a reduction of a large design where the
issue is that the output and the input of the a clock gate are tied together.
In the original, large design the hierarchy is much deeper than this.

FIRRTL version 5.2.0
circuit Foo:

  domain ClockDomain:

  extmodule ClockGate:
    input A: Domain of ClockDomain
    output B: Domain of ClockDomain
    input in: Clock domains [A]
    input en: UInt<1> domains [A]
    input out: Clock domains [B]

  extmodule Baz:
    input A: Domain of ClockDomain
    input clock: Clock domains [A]

  module Bar:
    input clock: Clock
    input en: UInt<1>

    ; Default instantiation:
    ;   1. instantiate `baz`
    ;   2. and connect `baz.clock`.
    inst baz of Baz
    connect baz.clock, clock ; <-------- Comment this out to pass inference.

    ; Now a clock gate is added:
    ;   1. clock gate the input `clock`
    ;   2. and _override_ connection to `baz.clock`.
    inst clockGate of ClockGate
    connect clockGate.in, clock
    connect clockGate.en, en
    connect baz.clock, clockGate.out

  public module Foo:
    input clock: Clock
    input en: UInt<1>

    inst bar1 of Bar
    inst bar2 of Bar

    connect bar1.en, en
    connect bar2.en, en

Running this produces:

# firtool -parse-only -domain-mode=infer-all Bar.fir
Bar.fir:43:5: error: 'firrtl.matchingconnect' op illegal domain crossing in operation
    connect bar2.en, en
    ^
Bar.fir:43:5: note: see current operation: "firrtl.matchingconnect"(%1#2, %arg1) : (!firrtl.uint<1>, !firrtl.uint<1>) -> ()
Bar.fir:40:5: note: 1st operand has domains: [ClockDomain: bar2.ClockDomain]
    inst bar2 of Bar
    ^
Bar.fir:37:11: note: 2nd operand has domains: [ClockDomain: bar1.ClockDomain]
    input en: UInt<1>
          ^

There are several problems that make this difficult to debug:
,

  1. The location of the error is at the highest connection, when the actual fix
    is much lower in the design. However, there InferDomains can't know that
    this is incorrect until the highest connection is made.

  2. The inferred domains make things confusing as bar2.ClockDomain and
    bar1.ClockDomain have no actual information about what they are or where
    they came from.

  3. It's unclear what action should be taken to rectify the error. This is
    admittedly hard as there are multiple possible solutions. However, the
    current error reporting almost implies that there is a missing
    unsafe_domain_cast on en in order to assign it to bar2.en. Yet, the
    domains are purely inferred at this point.

#10037 is a quick attempt to improve (2). However, this didn't prove much use
in my debugging. My actual debug loop here was to use circt-reduce with
certain reductions disabled to preserve more structure. The reduction was small
enough to then compare against the original circuit and check for errors.
circt-reduce helped here because it did things like inlining which moved the
clock gates closer to their usage site as opposed to having them be very deep in
the hierarchy.

Note 1: the underlying reason for the clock gate being connected to itself is
because InferDomains is running before ExpandWhens. The existence of this
circuit implies that our attempts to infer domains before resolving
last-connect-semantics is probably wrong.

Note 2: the fact that it is possible to connect a clock gate's output to its
input seems entirely wrong. This could be viewed as being a class of errors
that would be caught with domain cycle detection. This motivates being able to
track if an output domain on an external module is "synchronous to" an input
domain. (Does there exist a cycle from input to output? If so, then disallow
this.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    FIRRTLInvolving the `firrtl` dialect

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions