[FIRRTL] Don't force non-local trackers in Dedup.

[mikeurbach]

The previous behavior for local trackers would be to clone the tracker into duplicate copies for each unique instance. This will fail in LowerClasses, because one path op that used to refer to a single entity as a local reference would now refer to every hierarchical instance of that entity. One path can't refer to multiple things, so if the user specified a local reference, allow that to pass through without expanding it into every hierarchical instance.

[prithayan]

Looks good, but do we need to handle ambiguous paths in LowerClass because of this ?

[mikeurbach]

Nope, I don't think I don't think a local target is ambiguous. If a user made a path like Foo>bar:Bar, then they are referring to the instance bar within Foo. If Foo is instantiated multiple times, that's ok, this path would still refer to the instance bar within any of those instances of Foo, and lower classes is (now) happy to emit a local hw.hierpath in this case (i.e., the hierpath would be hw.hierpath [@Foo::@bar] with a single inner name ref).

The difference in this change is if Foo happened to dedupe with something else, this makeAnnotationsAbsolute logic would have duplicated the local tracker annotation once for each instance of Foo. By the time this hits lower classes, lower classes has to error out, because there is one path, which originally referred to the instance of bar within the module Foo, and now refers to the instance of bar with each instance of Foo. Effectively, the path was expanded from one reference into many, which we will fail on. So I think this change is fine from that perspective, if a single local path came in, this makes sure a single local path comes out.

[youngar]

How do we end up with duplicated local trackers, anyways? Maybe the inliner could do it?

[mikeurbach]

How do we end up with duplicated local trackers, anyways? Maybe the inliner could do it?

Likely inliner, potentially others. I need to chase this down and resolve it. I added dedupe of local trackers here in Dedup similar to how we do it for the local dont touch annotations, but in some testing downstream I'm seeing other sources of duplicated local trackers that I need to chase down.

[mikeurbach]

I think the last commit is still needed here. But the problem I was seeing downstream was actually an interaction with PrefixModules, which can also lead to duplicate annotations in cases like the examples here. That is more fundamental; I don't think there is anything we can do there with the current representation of trackers as annotations, so we'll need to hold this until module prefixes are applied by Chisel and not annotations. I do think we'll want this change at some point.

[mikeurbach]

And, now that I think about this, I may have been mistaken; the last commit here was probably not needed, and was me confusing the interaction with PrefixModules with the logic in Dedup.