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:

Description

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)
    (w_s1FA
       :: 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 ->
        $wa_s1G0
          @ 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 {
$s$wa_r1Hy
  :: forall s_aIE.
     GHC.Types.Int
     -> 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$wa_r1Hy
          @ s_aIE
          sc_s1Gq
          sc1_s1Gr
          sc2_s1Gs
          sc3_s1Gt
          sc4_s1Gu
          (GHC.Prim.+# sc5_s1Gv 1)
          sc6_s1Gw
          s2#1_aS8
        }
        };
      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 {
$wa_r1y9
  :: 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)
    (w_s1wz
       :: 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 ->
        $wa_r1y9
          @ s_aGn
          wild1_aPx
          wild11_aQm
          (GHC.Prim.+# ww_s1wD 1)
          ww1_s1wH
          s2#1_aQc
        }
        }
        }
        };
      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 7.0.1.20101215, 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  simonpj@microsoft.com
  * 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  simonpj@microsoft.com
       * 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

Merged.

Note: See TracTickets for help on using tickets.