llvm / llvm/circt

[FIRRTL] Expand Whens Doesn't Check firrtl.wire initialization of domain type

Open
#10,248 1 comment 0 reactions 0 assignees View on GitHub
bug FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

The following should error, but it doesn't:

```mlir
module {
firrtl.circuit "UndrivenWireSubfield" {
firrtl.domain @ClockDomain [#firrtl.domain.field<"source", !firrtl.string>]
firrtl.module @UndrivenWireSubfield() {
%0 = firrtl.string ""
%wire = firrtl.wire : !firrtl.domain<@ClockDomain(source: !firrtl.string)>
%1 = firrtl.domain.subfield %wire[source] : !firrtl.domain<@ClockDomain(source: !firrtl.string)>
%2 = firrtl.prop.eq %0, %1 : !firrtl.string
firrtl.property_assert %2, "hello" : !firrtl.bool
}
}
}
```

Currently, this will error out in `LowerClasses` if passed direclty.

``` console
# circt-opt Foo.mlir -pass-pipeline='builtin.module(firrtl.circuit(any(firrtl-expand-whens)))'
module {
firrtl.circuit "UndrivenWireSubfield" {
firrtl.domain @ClockDomain [#firrtl.domain.field<"source", !firrtl.string>]
firrtl.module @UndrivenWireSubfield() {
%0 = firrtl.string ""
%wire = firrtl.wire : !firrtl.domain<@ClockDomain(source: !firrtl.string)>
%1 = firrtl.domain.subfield %wire[source] : !firrtl.domain<@ClockDomain(source: !firrtl.string)>
%2 = firrtl.prop.eq %0, %1 : !firrtl.string
firrtl.property_assert %2, "hello" : !firrtl.bool
}
}
}
```

``` console
# circt-opt Foo.mlir -pass-pipeline='builtin.module(firrtl.circuit(firrtl-lower-domains,firrtl-lower-classes))'
Foo.mlir:12:13: error: uninitialized input port "source_in"
%wire = firrtl.wire : !firrtl.domain<@ClockDomain(source: !firrtl.string)>
^
```

I had originally thought that `LowerDomains` should unknown-initialize this. However, that doesn't make sense as there should be a precondition that everything is driven.

Contributor guide

No contributing guide indexed for this repository

Research direction

Reproduce the issue with the two circt-opt pass pipelines shown, then inspect firrtl-expand-whens and its interaction with firrtl-lower-domains and firrtl-lower-classes. Trace how the firrtl.wire domain subfield reaches LowerClasses and add or update coverage for the expected uninitialized-input error during expansion.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.