Opened 9 years ago

Closed 9 years ago

#4943 closed bug (fixed)

Another odd missed SpecConstr opportunity

Reported by: batterseapower Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:


Compiling the attached code with HEAD (and without case liberation) has this in the output of SpecConstr:

Rec {
$wa_s1G0 [Occ=LoopBreaker]
  :: forall s_aIU.
     Data.Array.Base.STUArray s_aIU GHC.Types.Int GHC.Types.Int
     -> GHC.Types.Int
     -> GHC.Prim.Int#
     -> GHC.Prim.Int#
     -> GHC.Prim.State# s_aIU
     -> (# GHC.Prim.State# s_aIU, () #)
[LclId, Arity=5, Str=DmdType LLLLL]
$wa_s1G0 =
  \ (@ s_aIU)
       :: Data.Array.Base.STUArray s_aIU GHC.Types.Int GHC.Types.Int)
    (w_s1FB :: GHC.Types.Int)
    (ww_s1FE :: GHC.Prim.Int#)
    (ww_s1FI :: GHC.Prim.Int#)
    (w_s1FK :: GHC.Prim.State# s_aIU) ->
    case GHC.Prim.># ww_s1FE ww_s1FI of wild_Xk [Dmd=Just A] {
      GHC.Types.False ->
        case w_s1FA
        of wild_aRL [Dmd=Just L]
        { Data.Array.Base.STUArray ds2_aRN [Dmd=Just U]
                                   ds3_aRO [Dmd=Just U]
                                   ds4_aRP [Dmd=Just U]
                                   marr#_aRQ [Dmd=Just L] ->
        case GHC.Prim.readIntArray# @ s_aIU marr#_aRQ ww_s1FE w_s1FK
        of wild2_aRX [Dmd=Just A]
        { (# s2#_aRZ [Dmd=Just L], e#_aS0 [Dmd=Just L] #) ->
        case w_s1FB
        of wild1_aSy [Dmd=Just L] { GHC.Types.I# y_aSA [Dmd=Just L] ->
        case GHC.Prim.writeIntArray#
               @ s_aIU marr#_aRQ ww_s1FE (GHC.Prim.+# e#_aS0 y_aSA) s2#_aRZ
        of s2#_aSp [Dmd=Just L] { __DEFAULT ->
          @ s_aIU w_s1FA w_s1FB (GHC.Prim.+# ww_s1FE 1) ww_s1FI s2#_aSp
      GHC.Types.True -> (# w_s1FK, GHC.Unit.() #)
end Rec }

i.e. we know the form of the first STUArray argument after the first iteration, but we leave it boxed and hence do the case every time around the loop.

Is constructor specialisation not meant to hit this case? It seems so simple that I feel that this problem must come from my misunderstanding rather than a compiler error.

Full command line:

ghc -fforce-recomp -c -dverbose-core2core -O2 -fno-liberate-case STUArray-Rewrite2.hs

(I disabled case liberation because I suspect it of having a bad effect elsewhere, but my understanding is that SpecConstr should still hit this case - IIRC all that CaseLib does that SpecConstr does not is deal with is hoisting case scrutinisation of the free variables of a function)

Note that this is a GHC version *with* the patch "Improve Simplifier and SpecConstr behaviour". I can reproduce this with the GHC 7.0 RC as well.

Attachments (1)

STUArray-Rewrite2.hs (783 bytes) - added by batterseapower 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by batterseapower

Attachment: STUArray-Rewrite2.hs added

comment:1 Changed 9 years ago by simonpj

Are you sure? Here's what I get with HEAD (-O2 -ddump-simpl). Looks good, except for the awkward felicity of passing unused arguments around the loop (#4941):

Rec {
  :: forall s_aIE.
     -> GHC.Types.Int
     -> GHC.Types.Int
     -> GHC.Prim.MutableByteArray# s_aIE
     -> GHC.Prim.Int#
     -> GHC.Prim.Int#
     -> GHC.Prim.Int#
     -> GHC.Prim.State# s_aIE
     -> (# GHC.Prim.State# s_aIE, () #)
[GblId, Arity=8, Caf=NoCafRefs, Str=DmdType LLLLLLLL]
$s$wa_r1Hy =
  \ (@ s_aIE)
    (sc_s1Gq :: GHC.Types.Int)
    (sc1_s1Gr :: GHC.Types.Int)
    (sc2_s1Gs :: GHC.Types.Int)
    (sc3_s1Gt :: GHC.Prim.MutableByteArray# s_aIE)
    (sc4_s1Gu :: GHC.Prim.Int#)
    (sc5_s1Gv :: GHC.Prim.Int#)
    (sc6_s1Gw :: GHC.Prim.Int#)
    (sc7_s1Gx :: GHC.Prim.State# s_aIE) ->
    case GHC.Prim.># sc5_s1Gv sc6_s1Gw of _ {
      GHC.Types.False ->
        case GHC.Prim.readIntArray# @ s_aIE sc3_s1Gt sc5_s1Gv sc7_s1Gx
        of _ { (# s2#_aRI, e#_aRJ #) ->
        case GHC.Prim.writeIntArray#
               @ s_aIE sc3_s1Gt sc5_s1Gv (GHC.Prim.+# e#_aRJ sc4_s1Gu) s2#_aRI
        of s2#1_aS8 { __DEFAULT ->
          @ s_aIE
          (GHC.Prim.+# sc5_s1Gv 1)
      GHC.Types.True -> (# sc7_s1Gx, GHC.Unit.() #)
end Rec }

comment:2 Changed 9 years ago by batterseapower

I *do* get that result if I disable case liberation. The issue is that it totally fails to happen if I turn case liberation off. I don't have a build of HEAD on this laptop, but the attached code compiles as follows with 6.12:

Rec {
  :: forall s_aGn.
     Data.Array.Base.STUArray s_aGn GHC.Types.Int GHC.Types.Int
     -> GHC.Types.Int
     -> GHC.Prim.Int#
     -> GHC.Prim.Int#
     -> GHC.Prim.State# s_aGn
     -> (# GHC.Prim.State# s_aGn, () #)
[GblId, Arity=5, Caf=NoCafRefs, Str=DmdType LLLLL]
$wa_r1y9 =
  \ (@ s_aGn)
       :: Data.Array.Base.STUArray s_aGn GHC.Types.Int GHC.Types.Int)
    (w1_s1wA :: GHC.Types.Int)
    (ww_s1wD :: GHC.Prim.Int#)
    (ww1_s1wH :: GHC.Prim.Int#)
    (w2_s1wJ :: GHC.Prim.State# s_aGn) ->
    case GHC.Prim.># ww_s1wD ww1_s1wH of _ {
      GHC.Bool.False ->
        case w_s1wz
        of wild1_aPx
        { Data.Array.Base.STUArray ds2_aPz ds3_aPA ds4_aPB marr#_aPC ->
        case GHC.Prim.readIntArray# @ s_aGn marr#_aPC ww_s1wD w2_s1wJ
        of _ { (# s2#_aPK, e#_aPL #) ->
        case w1_s1wA of wild11_aQm { GHC.Types.I# y_aQo ->
        case GHC.Prim.writeIntArray#
               @ s_aGn marr#_aPC ww_s1wD (GHC.Prim.+# e#_aPL y_aQo) s2#_aPK
        of s2#1_aQc { __DEFAULT ->
          @ s_aGn
          (GHC.Prim.+# ww_s1wD 1)
      GHC.Bool.True -> (# w2_s1wJ, GHC.Unit.() #)
end Rec }

This is bad. The command line was:

ghc -O2 -fno-liberate-case -ddump-simpl -c -fforce-recomp STUArray-Rewrite2.hs

comment:3 Changed 9 years ago by batterseapower

Sorry, read "disable case liberation" as "enable case liberation" above. (I don't have enough permissions to edit tickets, even my own contributions :-)

comment:4 Changed 9 years ago by batterseapower

Sorry, another correction: that result above is with, not 6.12. Apologies for Trac spam.

comment:5 Changed 9 years ago by simonpj

Status: newmerge

Aha. Thank you. You're dead right. How embarrassing.

Thu Feb  3 17:27:56 GMT 2011
  * Fix typo in SpecConstr that made it not work at all
  There was a terrible typo in this patch; I wrote "env"
  instead of "env1".
     Mon Jan 31 11:35:29 GMT 2011
       * Improve Simplifier and SpecConstr behaviour
  Anyway, this fix is essential to make it work properly.
  Thanks to Max for spotting the problem (again).

    M ./compiler/specialise/SpecConstr.lhs -1 +1

Please merge.

comment:6 Changed 9 years ago by igloo

Resolution: fixed
Status: mergeclosed


Note: See TracTickets for help on using tickets.