reduceNonRepPrim potentially introduces illegal variables

[martijnbastiaan]

Consider the primitive reduction of map:

clash-compiler/clash-lib/src/Clash/Normalize/PrimitiveReductions.hs

Lines 141 to 171 in 33047ee

	-- | Replace an application of the @Clash.Sized.Vector.map@ primitive on vectors
	-- of a known length @n@, by the fully unrolled recursive "definition" of
	-- @Clash.Sized.Vector.map@
	reduceMap
	:: TransformContext
	-> Integer -- ^ Length of the vector
	-> Type -- ^ Argument type of the function
	-> Type -- ^ Result type of the function
	-> Term -- ^ The map'd function
	-> Term -- ^ The map'd over vector
	-> NormalizeSession Term
	reduceMap (TransformContext is0 ctx) n argElTy resElTy fun arg = do
	tcm <- Lens.view tcCache
	let ty = termType tcm arg
	go tcm ty
	where
	go tcm (coreView1 tcm -> Just ty') = go tcm ty'
	go tcm (tyView -> TyConApp vecTcNm _)
	| (Just vecTc) <- lookupUniqMap vecTcNm tcm
	, [nilCon,consCon] <- tyConDataCons vecTc
	= do
	uniqs0 <- Lens.use uniqSupply
	fun1 <- constantPropagation (TransformContext is0 (AppArg Nothing:ctx)) fun
	let (uniqs1,(vars,elems)) = second (second concat . unzip)
	$ extractElems uniqs0 is0 consCon argElTy 'A' n arg
	funApps = map (fun1 `App`) vars
	lbody = mkVec nilCon consCon resElTy n funApps
	lb = Letrec (init elems) lbody
	uniqSupply Lens..= uniqs1
	changed lb
	go _ ty = error $ $(curLoc) ++ "reduceMap: argument does not have a vector type: " ++ showPpr ty

extractElems is called with is0: the InScopeSet of the current transformation. Given the pseudocode:

map (\x -> f 3 x) xs :: Vec 2 a

the primitive reduction might generate:

let 
  x = xs !! 0
  y = xs !! 1
in 
     (\x -> f 3 x) x 
  :> (\x -> f 3 x) y 
  :> Nil

This is incorrect as reduceNonRepPrim is potentially followed by an appProp:

clash-compiler/clash-lib/src/Clash/Normalize.hs

Lines 355 to 363 in fc8beb3

	flatten =
	innerMost (apply "appProp" appProp >->
	apply "bindConstantVar" bindConstantVar >->
	apply "caseCon" caseCon >->
	apply "reduceConst" reduceConst >->
	apply "reduceNonRepPrim" reduceNonRepPrim >->
	apply "removeUnusedExpr" removeUnusedExpr >->
	apply "flattenLet" flattenLet) !->
	topdownSucR (apply "topLet" topLet)

appProp has an invariant assuming:

clash-compiler/clash-lib/src/Clash/Normalize/Transformations.hs

Lines 1095 to 1097 in 02616d7

	-- 2. Ensure that @AppProp@ is only called in a context where there is no
	-- shadowing, i.e. the bindings can never never collide with the current
	-- inScopeSet.

reduceNonRepPrim should therefore make sure it doesn't introduce any new variables already mentioned in the body of fun1 (as well as the "normal" in scope set).