Module Duplication during Compilation

[fabianschuiki]

I'm toying around with ast::Compilation. I think I grasped that modules get monomorphized into instances, specifically InstanceSymbol::body. That made me wonder if slang just blindly creates a module copy for each instance, or whether it only creates one for every unique combination of parameters and interfaces.

[MikePopoloski]

Yes, currently one per instance. I previously had a cache to avoid creating new instance bodies when parameters and interface ports were the same but removed it when I put in a change to allow hierarchical references anywhere in parameter expressions (which is not legal SystemVerilog but I got a bunch of requests for it anyway since apparently the commercial tools mostly allow it). I still plan to go back and re-add the cache but it's somewhat complicated now to figure out when it's safe to cache an instance.

[fabianschuiki]

It always comes down to those tools accepting shady input and people starting to rely on it 😬

So from a slang-as-a-library user perspective, to only keep unique sets of instances, would it be safe to compare the definition, parameters, and interface ports, keep only one unique combination of each, and maybe just throw an error for the hierarchical references in parameters for now? Until slang itself regains its caching behavior.

[MikePopoloski]

Yes, that should be safe. In fact if you're consuming the slang AST then I think you don't need to worry about the hierarchical references in parameters problem -- you should not be able to observe an in-between state of the parameter values, so just uniquifying based on the param values (plus iface ports and definition) should be enough.

Actually it occurs to me that there was another problem with the cache, which was upward hierarchical name lookups. You might still run into problems there. I think I punted on thinking hard about how to account for those while still being able to cache instances.

The diff where I removed the cache is here in case it's helpful to you: 0892da3 (specifically the InstanceCache and CacheKey classes).

[fabianschuiki]

Awesome, thanks a lot for the pointer to that commit!

Ugh yeah I've also been running into and struggling with upward name references. It's almost like you want them to be exposed as a virtual port on the module itself, such that the elaborated module doesn't have to directly reference the thing in its parent, but it's carried up to the instance at the instantiation site… But that's pretty annoying if you need more than just the link to the upward reference, like the actual value for monomorphization :sad