[GHC] #14737: Improve performance of Simplify.simplCast

GHC ghc-devs at haskell.org
Wed Apr 4 13:05:20 UTC 2018


#14737: Improve performance of Simplify.simplCast
-------------------------------------+-------------------------------------
        Reporter:  tdammers          |                Owner:  (none)
            Type:  bug               |               Status:  patch
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  Compile-time      |  Unknown/Multiple
  performance bug                    |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #11735 #14683     |  Differential Rev(s):  Phab:D4385
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by goldfire):

 Replying to [comment:16 simonpj]:
 > If `arg` is `Refl` then `kind_co` is also `Refl`,

 Why do you say this? `kind_co` must be reflexive, but it needn't be
 `Refl`. Especially because it hasn't been optimized yet!

 > But perhaps spotting a `Refl` argument would be a bit more direct?

 I think it would.

 >
 > Meanwhile
 > * I agree that `kind_co` should be substituted.  But how?  By calling
 `ope_co4` on it?  Or `opt_co3`?  I don't understand the hierarchy of
 `opt_co` functions.

 You call the highest-numbered one for which you can meet the
 preconditions. In this case, that's `opt_co4` because we know `kind_co`'s
 role (nominal), and it should keep that role after optimization.

 > * I find that code for `extendLiftingContext` hard to grok.  In the
 `Refl` case we extend the `TvSubst`; otherwise we extend the `LiftCoEnv`.
 Very mysterious.  I looked at it a bit, but got lost in the deeply cryptic
 `liftEnvSubst`.

 You're right. This needs more documentation -- but a quick glance at this
 all suggests the code is currently correct.

 >
 > What we need, as usual, is to accumulate those arguments in a list;

 Yes, of course.

 > You mean that `Refl (forall a. ty)` can be regarded as a form of
 `ForAllCo`?  Especially since `mkForAllCo` goes to some trouble to build a
 `Refl` if it can.  So surely yes, `splitForAllCo_maybe` should split a
 `Refl (forall a.ty)`.

 Yes, that's exactly what I meant.

 >
 > > The only way InstCos can come into being is in the coercion optimizer.

 I was searching over a very old checkout of the codebase, and I wrongly
 assumed this hadn't changed. You're right of course.

 > Things to do
 > * Fix the missing substitution in `opt_co4`
 > * Fix `splitForAllCo_maybe`
 > * Fix `opt_co4` to behave well on deeply nsted `InstCos`
 > Might you do these -- you are more likely to get them right than me.

 OK. Hopefully can do in the next two weeks -- but I can't promise sooner.
 :(

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14737#comment:17>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler


More information about the ghc-tickets mailing list