patch applied (ghc): TickBox representation change

[Simon Peyton-Jones]

Andy

I had a look at what you did.   Generally looks good.  Much better than the Note version, as I hope you agree!   (You seem to have deleted quite a bit of code!)

Here's a qn.  I thought you were going to do ticks like this:
        dsExpr (HsTick a e) = tick{a} e
but instead you have
        dsExpr (HsTick a e) = case tick{a} of DEFAULT -> e
I think that both are probably fine, but the implications are not clear to me.  The former seems more robust somehow.  For example if we have
        case tick{a} of DEFAULT -> ....(case tick{a} of DEFAULT -> ...)
GHC might eliminate the inner tick.  Would that be OK (after all, you're only checking coverage, and it's covered)?

Clearly it's important not to eliminate the inner tick if it's for a different program point (tick{b}, say).  But I think that is ok because you make a fresh TickId for each {b} program point.  Still it'd be good to write this down.

Second point: (I'm offline at the moment): is there a Ghc Developer Wiki page describing how the implementation works?  I'd appreciate one.  You're replying on certain properties of the simplification and optimisation phases (such as not discarding (case tick of DEFAULT -> ...), which I'd like to have explicit.

Here's another qn.  You change
        if e then e1 else e2
into
        if (binTick (a,b) e) then e1 else e2

Then later, in CorePrep you go
        case (binTick (a,b) e) of True -> e1; False -> e2
==>  case e of True -> tick a e1; False -> tick b e2

So why did you not instead generate the 'tick' version in the first place?
        if e then (tick a e1) else (tick b e2)

It's perhpas not so obvious in the case of guards, because the then and else branches aren't neatly to hand.  But you can still do it.  Instead of

        binTick (a,b) e
generage
        case e of  True -> tick a True; False -> tick b False

Now the simplifier will do the rest for you.  In short, I think you might be able to eliminate BinTick altogether, which would surely be an advantage!



Simon

| -----Original Message-----
| From: cvs-all-bounces at haskell.org [mailto:cvs-all-bounces at haskell.org] On Behalf Of Andy Gill
| Sent: 29 November 2006 22:19
| To: cvs-ghc at haskell.org
| Subject: patch applied (ghc): TickBox representation change
|
| Wed Nov 29 14:09:57 PST 2006  andy at galois.com
|   * TickBox representation change
|
|   This changes the internal representation of TickBoxes,
|   from
|           Note (TickBox "module" n)  <expr>
|   into
|
|           case tick<module,n> of
|             _ -> <expr>
|
|   tick has type :: #State #World, when the module and tick numbe
|   are stored inside IdInfo.
|
|   Binary tick boxes change from
|
|            Note (BinaryTickBox "module" t f) <expr>
|
|   into
|
|             btick<module,t,f> <expr>
|
|   btick has type :: Bool -> Bool, with the module and tick number
|   stored inside IdInfo.
|
|
|     M ./compiler/basicTypes/Id.lhs +14
|     M ./compiler/basicTypes/IdInfo.lhs -7 +34
|     M ./compiler/basicTypes/MkId.lhs -1 +34
|     M ./compiler/basicTypes/Name.lhs +6
|     M ./compiler/coreSyn/CorePrep.lhs -19 +50
|     M ./compiler/coreSyn/CoreSyn.lhs -9
|     M ./compiler/coreSyn/CoreUtils.lhs -12 +5
|     M ./compiler/coreSyn/PprCore.lhs -15
|     M ./compiler/deSugar/Coverage.lhs -6 +12
|     M ./compiler/deSugar/DsUtils.lhs -2 +19
|     M ./compiler/iface/BinIface.hs -16
|     M ./compiler/iface/IfaceSyn.lhs -10
|     M ./compiler/iface/MkIface.lhs -3
|     M ./compiler/iface/TcIface.lhs -2
|     M ./compiler/main/DynFlags.hs -2 +2
|     M ./compiler/main/TidyPgm.lhs -7 +4
|     M ./compiler/simplCore/FloatIn.lhs -7
|     M ./compiler/simplCore/Simplify.lhs -10 +2
|     M ./compiler/stgSyn/CoreToStg.lhs -10 +4
|
| _______________________________________________
| Cvs-ghc mailing list
| Cvs-ghc at haskell.org
| http://www.haskell.org/mailman/listinfo/cvs-ghc