Re: [GHC] #14391: Make the simplifier independent of the typechecker

30 Apr 2018

      #14391: Make the simplifier independent of the typechecker
-------------------------------------+-------------------------------------
        Reporter:  nomeata           |                Owner:  ulysses4ever
            Type:  task              |               Status:  patch
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.3
      Resolution:                    |             Keywords:  newcomer
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D4503
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Description changed by ulysses4ever:

Old description:
...
I noticed that the simplifier module depends on all of the type checker,
and HsSyn stuff, and renamer stuff, which I found strange.
After a little investigation, it seems that the simplifier depends on
CoreMonad, and that pulls some very few type-checker related things:
1.
    {{{
    import TcRnMonad        ( initTcForLookup )
    import {-# SOURCE #-} TcSplice ( lookupThName_maybe )
    }}}
    for
    {{{
    thNameToGhcName :: TH.Name -> CoreM (Maybe Name)
    thNameToGhcName th_name = do
        hsc_env <- getHscEnv
        liftIO $ initTcForLookup hsc_env (lookupThName_maybe th_name)
    }}}
    which is not even used in GHC, but only in GHC Plugins, so this could
    probably be moved to a separate module pulled in by GhcPlugins.hs
2.
    {{{
    import TcEnv            ( lookupGlobal )
    }}}
    for
    {{{
    instance MonadThings CoreM where
        lookupThing name = do { hsc_env <- getHscEnv
                              ; liftIO $ lookupGlobal hsc_env name }
    }}}
    This might be a bit harder to disentangle. But if successful, it
would
    probably make building GHC in parallel quite a bit faster. And it
just
    seems strange to me that the Core-to-Core code should depend on the
    type checker…
Simon says:
...
Both of these code paths go through initTcForLookup which is massive
overkill, as the comment with `TcEnv.lookupGlobal` says.  There's clearly
a ToDo here to strip off the redundant stuff and do a minimal lookup.

...
I am optimistically marking this as `newcomer` because it is a
refactoring task, and a good way of learning a bit about various pieces,
with a reasonably clear notion of “success”.
----
**UPDATE** (April 2018, Phab:4503): both points described above are
addressed to the extent when in order to close the ticket we need to do
mere code movement. Namely.
1. `lookupThName_maybe` is eliminated from `CoreMonad.hs` completely. Its
client, though, `thNameToGhcName`, is better to be moved in the future
also, for it is not used in the `CoreMonad.hs` (or anywhere else) anyway.
Joachim suggested “any module reexported by GhcPlugins (or maybe even
that module itself)”.
2. `CoreMonad.hs` still calls `lookupGlobal` which is no longer bound to
the typechecker monad, but still resides in `TcEnv.hs` — it should be
moved out of Tc-land at some point in the future in order to close the
ticket.
New description:

 I noticed that the simplifier module depends on all of the type checker,
 and HsSyn stuff, and renamer stuff, which I found strange.

 After a little investigation, it seems that the simplifier depends on
 CoreMonad, and that pulls some very few type-checker related things:

 1.
     {{{
     import TcRnMonad        ( initTcForLookup )
     import {-# SOURCE #-} TcSplice ( lookupThName_maybe )
     }}}
     for
     {{{
     thNameToGhcName :: TH.Name -> CoreM (Maybe Name)
     thNameToGhcName th_name = do
         hsc_env <- getHscEnv
         liftIO $ initTcForLookup hsc_env (lookupThName_maybe th_name)
     }}}
     which is not even used in GHC, but only in GHC Plugins, so this could
     probably be moved to a separate module pulled in by GhcPlugins.hs

 2.
     {{{
     import TcEnv            ( lookupGlobal )
     }}}
     for
     {{{
     instance MonadThings CoreM where
         lookupThing name = do { hsc_env <- getHscEnv
                               ; liftIO $ lookupGlobal hsc_env name }
     }}}
     This might be a bit harder to disentangle. But if successful, it would
     probably make building GHC in parallel quite a bit faster. And it just
     seems strange to me that the Core-to-Core code should depend on the
     type checker…

 Simon says:
...
Both of these code paths go through initTcForLookup which is massive
 overkill, as the comment with `TcEnv.lookupGlobal` says.  There's clearly
 a ToDo here to strip off the redundant stuff and do a minimal lookup.
I am optimistically marking this as `newcomer` because it is a refactoring
 task, and a good way of learning a bit about various pieces, with a
 reasonably clear notion of “success”.

 ----

 **UPDATE** (April 2018, Phab:4503): both points described above are
 addressed to the extent when in order to close the ticket we need to do
 mere code movement. Namely.

 1. `lookupThName_maybe` and `initTcForLookup` are eliminated from
 `CoreMonad.hs` completely. Its client, though, `thNameToGhcName`, is
 better to be moved in the future also, for it is not used in the
 `CoreMonad.hs` (or anywhere else) anyway. Joachim suggested “any module
 reexported by GhcPlugins (or maybe even that module itself)”.

 2. `CoreMonad.hs` still calls `lookupGlobal` which is no longer bound to
 the typechecker monad, but still resides in `TcEnv.hs` — it should be
 moved out of Tc-land at some point in the future in order to close the
 ticket.

--

-- 
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14391#comment:25
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler