clash-lang / clash-compiler

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

caseOneAlt hides x-optimization #1892

Open christiaanb opened 3 years ago

christiaanb commented 3 years ago

https://github.com/clash-lang/clash-compiler/blob/a69ace9d21a6a1a925138588a428ed249d191641/clash-lib/src/Clash/Normalize/Transformations/Case.hs#L614-L618

imagine

f = \x y -> case x of {True -> expensive y; False -> expensive y}

if we had:

g = \y -> f undefined y

then we would first constant-specialize f:

g = \y -> f' y

f' = \y -> (\x y -> case x of {True -> expensive y; False -> expensive y) undefined y

which after regular constant propagation becomes:

f' = \y -> case undefined of {True -> expensive y; False -> expensive y}

now, because x-optimization runs late

https://github.com/clash-lang/clash-compiler/blob/a69ace9d21a6a1a925138588a428ed249d191641/clash-lib/src/Clash/Normalize/Strategy.hs#L33-L40

caseOneAlt (part of caseCon) will fire first:

f' = \y -> expensive y

while if xoptimization ran first we would've had:

f' = \y -> undefined

I guess a solution is for caseOneAlt to instead of

case e of {p -> alt; other_alts} ==>  alt

do

case e of {p -> alt; other_alts}  ==> case e of {p -> alt}

for the case where caseOneAlt eliminates the case-expression where all alternatives are equal. Then during the x-optimization pass we can cleanup:

case e of {p -> alt} ==> alt
leonschoorl commented 3 years ago

Transforming case undefined of {...} ==> undefined isn't done by xOptimize, but by caseCon'. And caseCon' run just after caseOneAlt: https://github.com/clash-lang/clash-compiler/blob/8894dd63aae5522845b7e37ab3c0d6cf10a084a8/clash-lib/src/Clash/Normalize/Transformations/Case.hs#L124

Flipping that order works for simple cases. (and changing >-! into >->)

And caseOneAlt is still used directly in some other transformations.