Add name prefixes when functions are inlined

[christiaanb]

We need to add name prefixes when functions are inlined. I.e. when we have

{-# ANN f Synthesize … #-}
f a b = ... g ... 

g p q = ... setName @”foo” h ... 

{-# ANN h Synthesize … #-}
h x y = ...

The instance of ‘h’ inside the ‘f’ module is currently called ‘foo’, note that there will be no ‘g’ module as it’s inlined because it doesn’t have a synthesize or noinline pragma. What we still need to do is that when ‘g’ is inlined, all the instance labels are prefixed with “g_”, so that the instance of ‘h’ in ‘f’ is called “g_foo”.

implementation suggestion

• Tick the inlined expression with a NameMod PrefixName (LitTy (SymTy <name_of_inlined_function>)), make sure to remove all module name qualifiers from the name though.
• Locations where we're currently inlining:

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

    Lines 318 to 340 in db19778

    	(newUsed,il_ct) <- partitionEithers <$> mapM flattenNode flattenedUsed
    	let (toInline,il_used) = unzip il_ct
    	subst = extendGblSubstList (mkSubst emptyInScopeSet) toInline
    	newExpr <- case toInline of
    	[] -> return tm
    	_ -> do
    	-- To have a cheap `appProp` transformation we need to
    	-- deshadow, see also Note [AppProp no-shadow invariant]
    	let tm1 = deShadowTerm emptyInScopeSet (substTm "flattenCallTree.flattenExpr" subst tm)
    	#ifdef HISTORY
    	-- NB: When HISTORY is on, emit binary data holding the recorded rewrite steps
    	let !_ = unsafePerformIO
    	$ BS.appendFile "history.dat"
    	$ BL.toStrict
    	$ encode RewriteStep
    	{ t_ctx = []
    	, t_name = "INLINE"
    	, t_bndrS = showPpr (varName nm')
    	, t_before = tm
    	, t_after = tm1
    	}
    	#endif
    	rewriteExpr ("flattenExpr",flatten) (showPpr nm, tm1) (nm', sp)

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

    Lines 347 to 354 in db19778

    	let (toInline',allUsed') = unzip (map goCheap allUsed)
    	subst' = extendGblSubstList (mkSubst emptyInScopeSet)
    	(Maybe.catMaybes toInline')
    	-- To have a cheap `appProp` transformation we need to
    	-- deshadow, see also Note [AppProp no-shadow invariant]
    	let tm1 = deShadowTerm emptyInScopeSet (substTm "flattenCallTree.flattenCheap" subst' newExpr)
    	newExpr' <- rewriteExpr ("flattenCheap",flatten) (showPpr nm, tm1) (nm', sp)
    	return (CBranch (nm,(nm',sp,inl,newExpr')) (concat allUsed'))

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

    Line 368 in db19778

    inlineNonRep :: HasCallStack => NormRewrite

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

    Line 949 in db19778

    inlineWorkFree :: HasCallStack => NormRewrite

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

    Line 1011 in db19778

    inlineSmall :: HasCallStack => NormRewrite

  • clash-compiler/clash-lib/src/Clash/Core/Evaluator.hs

    Lines 357 to 360 in db19778

    	Nothing -> case lookupVarEnv x' gbl of
    	Just e | isGlobalId x'
    	-> Just (Heap gh (GPureHeap (delVarEnv gbl x')) h ids is,GUpdate x':k,e)
    	_ -> Nothing

[alex-mckenna]

Accidentally closed this issue instead of the related one. Whoops.

[alex-mckenna]

@christiaanb Where is the inlining being performed in Clash.Core.Evaluator? I can't quite make sense of what's being inlined in the snippet you linked.

[christiaanb]

clash-compiler/clash-lib/src/Clash/Core/Evaluator.hs

Lines 357 to 360 in db19778

	Nothing -> case lookupVarEnv x' gbl of
	Just e | isGlobalId x'
	-> Just (Heap gh (GPureHeap (delVarEnv gbl x')) h ids is,GUpdate x':k,e)
	_ -> Nothing

That e is the body of the global function x'; so we're basically inlining x'.

[alex-mckenna]

So interestingly enough, the changes are in and the example in the issue now works as expected. However, the changes to the inlining functions in the Transformations module results in some tests failing for seemingly unrelated reasons:

• Normalisation resulting in a recursive let binding (e.g. Feedback/Fib.hs)
• Generated HDL has name clashes sometimes (e.g. LambdaDrop)

[christiaanb]

Feedback/Fib.hs depends on recToLetrec firing correctly; but that transformation is fragile.

[alex-mckenna]

That's good, gives me a place to start looking. With LambdaDrop it's generating a name clash between a port and a signal in the core module (prior to adding the NamePrefix tick it correctly changed the signal name from x to x_0).

@leonschoorl and @martijnbastiaan pointed me towards #476 which seems closely related

[alex-mckenna]

Another failing test I didn't spot before: CustomReprs.Indexed

topentity.vhdl:16:11: character '$' can only be used in strings or comments
topentity.vhdl:16:12: ':' is expected instead of "case_alt"
topentity.vhdl:16:20: missing ";" at end of object declaration
topentity.vhdl:33:6: character '$' can only be used in strings or comments
topentity.vhdl:33:7: '<=' is expected instead of "case_alt"
topentity.vhdl:41:22: character '$' can only be used in strings or comments
topentity.vhdl:41:22: ';' expected at end of signal assignment
topentity.vhdl:41:22: (found: an identifier)
topentity.vhdl:41:32: '<=' is expected instead of 'when'
topentity.vhdl:41:32: unexpected token 'when' in a primary

I would suspect it's generating names with $ in it which aren't being escaped (although I can't seem to get it to run with cabal exec clash -- ...)

[christiaanb]

That's related to the topLet firing too early as well.

[christiaanb]

Implemented in: #958