Opened 9 months ago

Closed 9 months ago

Last modified 8 months ago

#16029 closed bug (fixed)

Inadequate absence analysis

Reported by: simonpj Owned by:
Priority: normal Milestone:
Component: Compiler Version: 8.6.2
Keywords: DemandAnalysis Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: stranal/should_compile/T16029
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Consider

data S = MkS Int Int
g1 :: S -> Int -> Int
g1 (MkS x y) 0 = 0
g1 (MkS x y) n = g1 (MkS y x) (n-1)

With GHC 8.6 we get

$wg1 :: S -> GHC.Prim.Int# -> GHC.Prim.Int#
[Str=<S,1*H><S,1*U>]
$wg1 = \ (w_s2oH :: S)
         (ww_s2oL :: GHC.Prim.Int#) ->
         case w_s2oH of { MkS x_s2pz y_s2pA ->
         case ww_s2oL of ds_X2nb {
           __DEFAULT ->
             Foo.$wg1 (Foo.MkS y_s2pA x_s2pz) (GHC.Prim.-# ds_X2nb 1#);
           0# -> 0# }}

g1 :: S -> Int -> Int
[Str=<S,1*H><S(S),1*U(1*U)>m]
g1 = \ (w_s2oH :: S) (w1_s2oI :: Int) ->
       case w_s2oH  of w2_X2pG { MkS ipv_s2p2 ipv1_s2p3 ->
       case w1_s2oI of { GHC.Types.I# ww1_s2oL ->
       case Foo.$wg1 w2_X2pG ww1_s2oL of ww2_s2oP { __DEFAULT ->
       GHC.Types.I# ww2_s2oP }}}

What terrible code! We evaluate the S argument in the wrapper, and box and unbox it every time around the loop, even though it is never ultimately used.

Here's what happens if the arguments are banged:

data T = MkT !Int !Int
g2 :: T -> Int -> Int
g2 (MkT x y) 0 = 0
g2 (MkT x y) n = g2 (MkT y x) (n-1)

We get

$wg2 GHC.Prim.Int# -> GHC.Prim.Int# -> GHC.Prim.Int# -> GHC.Prim.Int#
[Str=<L,1*U><L,1*U><S,1*U>]
Foo.$wg2 = \ (ww_s2ow :: GHC.Prim.Int#)
             (ww1_s2ox :: GHC.Prim.Int#)
             (ww2_s2oB :: GHC.Prim.Int#) ->
             case ww2_s2oB of ds_X2n0 {
               __DEFAULT -> Foo.$wg2 ww1_s2ox ww_s2ow (GHC.Prim.-# ds_X2n0 1#);
               0# -> 0# }

g2 :: T -> Int -> Int
[Str=<S(SS),1*U(1*U,1*U)><S(S),1*U(1*U)>m ]
g2 = \ (w_s2os :: T) (w1_s2ot :: Int) ->
       case w_s2os  of { MkT ww1_s2ow ww2_s2ox ->
       case w1_s2ot of { GHC.Types.I# ww4_s2oB ->
       case Foo.$wg2 ww1_s2ow ww2_s2ox ww4_s2oB of ww5_s2oF {
         __DEFAULT -> GHC.Types.I# ww5_s2oF }}}

Still terrible. We pass the two components around the loop before discarding them at the end.

Change History (4)

comment:1 Changed 9 months ago by bgamari

Milestone: 8.6.3

Ticket retargeted after milestone closed

comment:2 Changed 9 months ago by Simon Peyton Jones <simonpj@…>

In d77501cd/ghc:

Improvements to demand analysis

This patch collects a few improvements triggered by Trac #15696,
and fixing Trac #16029

* Stop making toCleanDmd behave specially for unlifted types.
  This special case was the cause of stupid behaviour in Trac
  #16029.  And to my joy I discovered the let/app invariant
  rendered it unnecessary.  (Maybe the special case pre-dated
  the let/app invariant.)

  Result: less special-case handling in the compiler, and
  better perf for the compiled code.

* In WwLib.mkWWstr_one, treat seqDmd like U(AAA).  It was not
  being so treated before, which again led to stupid code.

* Update and improve Notes

There are .stderr test wibbles because we get slightly different
strictness signatures for an argumment of unlifted type:
    <L,U> rather than <S,U>        for Int#
    <S,U> rather than <S(S),U(U)>  for Int

comment:3 Changed 9 months ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: stranal/should_compile/T16029

Fixed, but the test isn't being run -- see Trac #16042

comment:4 Changed 8 months ago by simonpj

Keywords: DemandAnalysis added
Note: See TracTickets for help on using tickets.