microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

More powerful use of `mkAtomicNoFVs` OK? #792

Open toelli-msft opened 3 years ago

toelli-msft commented 3 years ago

mkAtomicNoFVs is very useful because it provides a cheap way of preserving sharing whilst using a Haskell-level function to do "substitution" into an Expr. It comes with the caveat

we use the same variable name every time. That's not safe in general, but the bodys we use never mention any other variables, it's fine

However, it would be helpful if the bodys could bind variables! (In particular it would help Hybrid shapes https://github.com/microsoft/knossos-ksc/pull/766). I think that NB3 below is correct, isn't it? (this mkAtomicNoFVs is the same as in master but with a different variable name).

@simonpj Is there a reason we wouldn't want to use mkNoAtomicFVs like this? Perhaps it's too delicate? But it is very useful!

-- (mkAtomicNoFVs e body) returns the expression (let a = e in body a)
-- where body :: TExpr -> TExpr is a function expecting an expression
-- The idea is that body might use its argument many types, and we
-- don't want to duplicate e, so we let-bind it instead
--
-- NB1: there's a short-cut when e is trivial (e.g. another variable)
--
-- NB2: we use the same variable name every time.  That's not safe in
--      general, but if body never mention any other variables,
--      it's fine c.f. the more general Opt.makeAtomic, which does
--      not have this side condition but which requires an in-scope
--      set
--
-- NB3: However, if you want to use bodys that bind variables then
-- just make sure they never bind ksc$mkAtomicVar and all will be fine.
mkAtomicNoFVs :: TExpr -> (TExpr -> TExpr) -> TExpr
mkAtomicNoFVs e body
  | isTrivial e = body e
  | otherwise   = mkLet ev e (body (Var ev))
  where
    ev = TVar (typeof e) (Simple ("ksc$mkAtomicVar"))
simonpj commented 3 years ago

There are two potential gotchas to do with capturing.

So long as body neither mentions no binds ksc$mkAtomicVar we are fine.