Keep recursive and non-recursive let in Clash Core

[alex-mckenna]

In GHC core, the difference between recursive and non-recursive binders is preserved. In Clash, this difference is not preserved in let expressions / the bindings map, which leads to sometimes needing to calculate the connected components of the bindings.

However, since Letrec is used a lot (i.e. in normalization) it is painful to simply differentiate everywhere immediately. As a compromise, this is kept as a pattern synonym so existing code should work as it did before. In the future we can migrate these cases until only Let NonRec{} and Let Rec{} are used in code. Until this time, the assumption is made that NonRec is always non-recursive, but Rec holds recursive and potentially recursive bindings.

The parts of the compiler where it is easier to change the definitions now have been changed.

Still TODO:

• Write a changelog entry (see changelog/README.md)
• Check copyright notices are up to date in edited files

[christiaanb]

According to https://ghc.gitlab.haskell.org/ghc/doc/users_guide/exts/pattern_synonyms.html you shouldn't need the PatternSynonyms extension when you use them, only when you define them. Is that not true?

[christiaanb]

Suggested change

	deriving (Eq, Show, Generic, NFData, Hashable, Binary, Functor)
	deriving (Show, Generic, NFData, Hashable, Binary, Functor)
	-- | Structural equivalence, not alpha equivalence
	deriving instance Eq a => Eq (Bind a)

[alex-mckenna]

I've left a comment by the deriving clause but otherwise left this. I think it would be a good idea to deal with structural / alpha equality for Bind in #1990, because I also have a similar thing with alpha-equivalence for patterns in #1976 that I want to move to that PR

[christiaanb]

I don't want to add the Structural and Alpha newtypes suggested in #1987 to #1990 though, I want to do that in a separate PR. #1990 significantly reduces compile times which I don't want to delay merging just because there's a further improvement on the horizon, here's master vs #1990

christiaan@DESKTOP-O5QKI8I:~/clash-compiler$ time cabal run clash-testsuite -- -p VHDL.clash --hide-successes -j6
Up to date

All 378 tests passed (555.68s)

real    9m17.383s
user    54m37.413s
sys     2m58.446s

christiaan@DESKTOP-O5QKI8I:~/clash-compiler$ time cabal run clash-testsuite -- -p VHDL.clash --hide-successes -j6
Up to date

All 378 tests passed (410.44s)

real    6m51.604s
user    38m45.764s
sys     2m21.107s

[alex-mckenna]

Right, but you add the eqTerm function, and I want to add Bind to that and remove the TODO (since I also want to dig out the alpha / structural equality on Pat and add it to that PR anyway we may as well do both)

[christiaanb]

I don't want to add the Structural and Alpha newtypes suggested in #1987 to #1990 though, I want to do that in a separate PR. #1990 significantly reduces compile times which I don't want to delay merging just because there's a further improvement on the horizon, here's master vs #1990

christiaan@DESKTOP-O5QKI8I:~/clash-compiler$ time cabal run clash-testsuite -- -p VHDL.clash --hide-successes -j6
Up to date

All 378 tests passed (555.68s)

real    9m17.383s
user    54m37.413s
sys     2m58.446s

christiaan@DESKTOP-O5QKI8I:~/clash-compiler$ time cabal run clash-testsuite -- -p VHDL.clash --hide-successes -j6
Up to date

All 378 tests passed (410.44s)

real    6m51.604s
user    38m45.764s
sys     2m21.107s