[FIRRTL] Dedup: accidentally processes modules before referenced classes

[youngar]

firrtl.circuit "Top" {
  firrtl.module @Top() {
    firrtl.instance foo @Foo()
    firrtl.instance bar @Bar()
  }

  firrtl.module private @Foo() {
    %w = firrtl.wire : !firrtl.class<@Spam()>
  }
  firrtl.module private @Bar() {
    %w = firrtl.wire : !firrtl.class<@Eggs()>
  }

  firrtl.class @Spam() {}
  firrtl.class @Eggs() {}
}

Running bin/circt-opt --firrtl-dedup dedup.mlir --debug-only=firrtl-dedup --mlir-disable-threading gives, with an extra printf in referred module remapping, gives:

===- Dedup circuit "Top" ----------------------------------------------------===

Found 5 modules
Computing module information
- Hash af840e70c3ea306c15884599064c0c8327f8c7f7b2a4c77b5d07495fd317fa85 for "Foo"
- Hash af840e70c3ea306c15884599064c0c8327f8c7f7b2a4c77b5d07495fd317fa85 for "Bar"
- Hash 68df36fc78303189fb153e739d3b1646133652308976e53e12d57107e421f5aa for "Top"
- Hash 27acbd3d013a60709d80df46557bfb174c0ac9eeac2e74887f6137d8ac43b62b for "Spam"
- Hash 27acbd3d013a60709d80df46557bfb174c0ac9eeac2e74887f6137d8ac43b62b for "Eggs"
Update modules
referred module: "Spam"
renamed to module: <<NULL ATTRIBUTE>>
- No match for "Foo"
referred module: "Eggs"
renamed to module: <<NULL ATTRIBUTE>>
- Replace "Bar" with "Foo"
referred module: "Bar"
renamed to module: "Foo"
referred module: "Foo"
renamed to module: "Foo"
- No match for "Top"
- No match for "Spam"
- Cannot dedup "Eggs" with "Spam" because both are public
Update annotations
- Updating 2 symbol-sensitive ops in "Top"
- Updating 1 symbol-sensitive ops in "Foo"
module {
  firrtl.circuit "Top" {
    firrtl.module @Top() {
      firrtl.instance foo @Foo()
      firrtl.instance bar @Foo()
    }
    firrtl.module private @Foo() {
      %w = firrtl.wire : !firrtl.class<@Spam()>
    }
    firrtl.class @Spam() {
    }
    firrtl.class @Eggs() {
    }
  }
}

Since Spam and Eggs did not deduplicate, due to both being public, Foo and Bar should not have deduplicated.

When hashing a module, if the hasher sees a class type, it records the class in its list of "referred modules".  Dedup depends on visiting the modules in post-order, so we can remap a module's "referred modules" (children modules) and successfully deduplicate parent modules when the children modules have deduped.  I don't think the instance graph takes into account a (e.g.) port of class type to consider that class as a child module.  The end result is that when we look this class up in the dedupMap before it has been processed, we get a null attribute, and since null == null we can deduplicate things we shouldn't.

[youngar]

When we fix this issue (or remove class deduplication), we should add this assert to dedup:

      // Replace module names referred in the module with new names.
      for (auto &referredModule : moduleInfo.referredModuleNames) {
        referredModule = dedupMap[referredModule];
        assert(referredModule && "all children should have been visited");
      }

Adding it causes the existing dedup.mlir test suite to fail.