clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.4k stars 149 forks source link

Move `DomainMap` from `ClashDesign` to `ClashEnv` #2554

Closed martijnbastiaan closed 11 months ago

martijnbastiaan commented 11 months ago

I would like to have this for an upcoming clash-cores primitive. Given that the 1.8 release is coming up, it would be great to have it in there.

Still TODO:

alex-mckenna commented 11 months ago

The new envDomains is the same as designDomains in ClashDesign. If you think it should be in ClashEnv instead I would also remove it from ClashDesign since there's no point having both.

I also wonder why we don't keep the HDL we're generating in ClashOpts, since we keep the vendor tooling we're targeting in there. I think it would make sense to have both in the same place

martijnbastiaan commented 11 months ago

I don't think ClashDesign is supposed to be accessible by blackboxes? In that case I do think make sense to move it ClashEnv.

As for the HDL: makes sense to be to store it in ClashOpts instead.

Thanks!

alex-mckenna commented 11 months ago

IIRC I made the split between what ended up in Env and Design pretty arbitrarily, because I was trying to make two compromises which don't really line up:

So overall the types don't useful convey much in practice. I did intend to play around with the first point more, because if Clash spent too long in GC for a design it could be useful to see if putting these non-changing, effectively static lifetime things (from the perspective of clash-lib) in a compact region to shorten heap traversals

martijnbastiaan commented 11 months ago

I also wonder why we don't keep the HDL we're generating in ClashOpts, since we keep the vendor tooling we're targeting in there. I think it would make sense to have both in the same place

After trying to move it to ClashOpts I think I know why: it's used in type level constructs (Backend (s :: HDL)). I.e., moving it to ClashOpts would mean storing a GADT there or risking split brain. I'll scrap it from this PR; strictly HDL is already accessible for black boxes, it's just grimy to get it. I'll introduce a convenience function when I need it.