[OM] parsing depend on the ordering of Class

[sequencer]

Chisel provides a unsafe version of get class type: Class.unsafeGetClassTypeByName, this is used when user need a Class Type w/o get the Scala val of which. This will cause an issue: I guarantee a Class Foo will be defined after this Class Bar, so I use it at Foo use the unsafe API, see:

FIRRTL version 4.0.0
circuit Bar :
  class Bar :
    output foo :Inst<Foo>
  class Foo :
    skip

However firtool complains:

> firtool bug.fir
bug.fir:4:22: error: unknown class 'Foo'
    output foo :Inst<Foo>

Proposing checking the unknown class at the last of parser, or create a new pass for validation.

[uenoku]

Yeah I think we shouldn't reject this (actually the same error would happen for type aliases as well). This might be tricky because currently class types on ports are computed form classOp itself so it's hard to fix when there are classes that reference each other on ports.

e.g.

FIRRTL version 4.0.0
circuit Bar :
  module Bar :
    input foo :Inst<Foo>
  module Foo :
    input foo :Inst<Bar>

[sequencer]

Is possible to delay the classOp construction?

[mikeurbach]

This is a shortcoming of the current FIRRTL parser for classes, which is taking a single pass in lexicographic order.

So for the short term, if you know you're making Foo, and you're making a Bar with a reference to Foo, you can try to re-arrange your generator to produce Foo before Bar.

Another option is to type erase the instance type in Bar. Rather than a Class.unsafeGetClassTypeByName("Foo"), you can construct a Property[AnyClassType]. You are allowed to connect a reference to Foo to a Property[AnyClassType], and this will be handled transparently in FIRRTL and the OM dialect evaluator. This doesn't allow you to access fields of the foo, because if the type is "any", we don't know what fields are legal. But if that is not required, that's another short term option.

The longer term option to handle this in the parser would probably require taking multiple passes through the textual FIRRTL to collect all the class names before parsing the class bodies. This is what we do for modules, so things like this are legal:

circuit Bar :
  module Bar :
    inst foo of Foo
  module Foo :
    skip

[dtzSiFive]

Yes, cannot use type aliases or classes defined later in the file.

Modules we parse their signatures in one pass and then the bodies after -- this approach doesn't work for type aliases or classes which each may contain in their signature references to other types (aliased or classes). For this reason, module signatures also must only use types (aliased, classes, so on) defined before that point in the file.

We parse directly into typed IR and rely on this for diagnostics (e.g., accessing a field of a type that isn't a bundle or is but doesn't have one of that name).

Def-before-use policy also avoids various sorts of cycles -- mutually recursive type alias, so on -- that would need to be checked for otherwise.

I'm not sure this can be accommodated without some significant changes + overhead re:tracking/indirection and if cannot be done in a single pass (+pre-pass for top-level declarations) should be considered carefully. Mostly thinking about type aliases -- but probably class ref types too?

(BTW - do we really parse class names in lexicographic order and not the order they appear in the file? (!))

[mikeurbach]

I'm not sure this can be accommodated without some significant changes + overhead re:tracking/indirection and if cannot be done in a single pass (+pre-pass for top-level declarations) should be considered carefully.

Yeah, this would need to be thought through carefully if we want to support this. Unlike modules, which can't have other modules in their signature, type aliases and object references can show up in signatures of modules and classes.

do we really parse class names in lexicographic order and not the order they appear in the file?

Sorry, I think I meant lexical order. What I mean is top to bottom in the order they appear in the file.