[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