Opened 5 months ago

Closed 7 weeks ago

#8569 closed bug (fixed)

ASSERT in testcase type-rep, only in some ways:

Reported by: nomeata Owned by:
Priority: normal Milestone:
Component: Test Suite Version: 7.7
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: gadt/type-rep Blocked By:
Blocking: Related Tickets:

Description

On HEAD, I currently (d55cc30658ae7c5afc0d62f3bd118fb2a5fcee40) get

=====> type-rep(normal) 2696 of 3834 [0, 0, 0] 
=====> type-rep(hpc) 2696 of 3834 [0, 0, 0] 
Compile failed (status 256) errors were:
[1 of 1] Compiling Main             ( type-rep.hs, type-rep.o )
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.7.20131124 for x86_64-unknown-linux):
	ASSERT failed! file compiler/basicTypes/Demand.lhs, line 439

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for type-rep(hpc)
=====> type-rep(optasm) 2696 of 3834 [0, 1, 0] 
Compile failed (status 256) errors were:
[1 of 1] Compiling Main             ( type-rep.hs, type-rep.o )
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.7.20131124 for x86_64-unknown-linux):
	ASSERT failed! file compiler/basicTypes/Demand.lhs, line 439

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for type-rep(optasm)
=====> type-rep(ghci) 2696 of 3834 [0, 2, 0] 
=====> type-rep(threaded1) 2696 of 3834 [0, 2, 0] 
=====> type-rep(threaded2) 2696 of 3834 [0, 2, 0] 
Compile failed (status 256) errors were:
[1 of 1] Compiling Main             ( type-rep.hs, type-rep.o )
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.7.20131124 for x86_64-unknown-linux):
	ASSERT failed! file compiler/basicTypes/Demand.lhs, line 439

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for type-rep(threaded2)
=====> type-rep(dyn) 2696 of 3834 [0, 3, 0] 
Compile failed (status 256) errors were:
[1 of 1] Compiling Main             ( type-rep.hs, type-rep.o )
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.7.20131124 for x86_64-unknown-linux):
	ASSERT failed! file compiler/basicTypes/Demand.lhs, line 439

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for type-rep(dyn)
=====> type-rep(optllvm) 2696 of 3834 [0, 4, 0] 
Compile failed (status 256) errors were:
[1 of 1] Compiling Main             ( type-rep.hs, type-rep.o )
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.7.20131124 for x86_64-unknown-linux):
	ASSERT failed! file compiler/basicTypes/Demand.lhs, line 439

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for type-rep(optllvm)

Change History (15)

comment:1 Changed 5 months ago by nomeata

That should be 1df2116c221941ef40a0f6f8fb7dcc42c56738e7 (changed due to pull rebase).

comment:2 Changed 5 months ago by nomeata

I have a freshly cleaned and built master checkout here, and it is reproducible again (using make -C testsuite TEST="type-rep")

Note that this patch is marked as skip() when running make -C testsuite fast.

(In case this looks like a soliloquy: I replied to a remark I got by mail, confusing it with a trac notification... ;-))

Last edited 5 months ago by nomeata (previous) (diff)

comment:3 Changed 5 months ago by nomeata

Minimalizing the example yields

module Example (x) where

data Rep t where
  Rint :: Rep Int
  Rdata :: Rep i -> (t -> i) -> (i -> t) -> Rep t

addUp :: Rep a -> a -> Int
addUp Rint          n         = n
addUp (Rdata i f g) x         = addUp i (f x)

x = addUp undefined [1]

Again, the problem only occurs with -O.

comment:4 Changed 5 months ago by nomeata

I believe the problem is a mis-calculated demand for addUp:

Rec {
Example.addUp [Occ=LoopBreaker]
  :: forall a_aqp. Example.Rep a_aqp -> a_aqp -> GHC.Types.Int
[LclIdX,
 Arity=2,
 Str=DmdType <S,1*U><L,U(U)>,
 Unf=Unf{Src=<vanilla>, TopLvl=True, Arity=2, Value=True,
         ConLike=True, WorkFree=True, Expandable=True,
         Guidance=IF_ARGS [30 0] 70 0}]
Example.addUp =
  \ (@ a_aBI)
    (ds_dBY [Dmd=<S,1*U>] :: Example.Rep a_aBI)
    (n_aqq [Dmd=<L,U(U)>] :: a_aBI) ->
    case ds_dBY of _ {
      Example.Rint dt_dC2 [Dmd=<L,A>] ->
        n_aqq `cast` (Sub dt_dC2 :: a_aBI ~# GHC.Types.Int);
      Example.Rdata @ i_aBR i_aqr [Dmd=<S,1*U>]
                    f_aqs [Dmd=<L,1*C1(U(U))>] g_aqt [Dmd=<L,A>] ->
        Example.addUp @ i_aBR i_aqr (f_aqs n_aqq)
    }
end Rec }

The demand on its first argument, U(U), clearly comes from the Rint case (where it is true), but in the other case this demand does not even type-check, as a can be some other type. So what goes wrong here?..

comment:5 Changed 5 months ago by nomeata

Here we go:

dmdAnal:Case2
    scrut ds_dBX{v} [lid]
    scrut_ty DmdType {dBX-><S,1*U>}
    alt_tys [DmdType {aqq-><S,1*U(U)>}, DmdType {aqq-><L,U>}]
    alt_ty DmdType {aqq-><L,U(U)>}
    res_ty DmdType {aqq-><L,U(U)> dBX-><S,1*U>}

If I read this correct, in human terms, this tell me
“The first case branch will use aqq (=n) at most once, and will take the box apart and use what’s inside, while do not know anything about how the right case branch uses n. The scrutinee does not talk about n at all. Therefore, the whole expression uses n in an unknown number of times, and also uses what’s inside”.

This sounds wrong to me (but I’m not firm on reading demand signatures yet).

comment:6 Changed 5 months ago by nomeata

Tracing this further I find this equation:

lubUse :: UseDmd -> UseDmd -> UseDmd
[..]
lubUse (UProd ux1) (UProd ux2)
     | length ux1 == length ux2    = UProd $ zipWith lubMaybeUsed ux1 ux2
     | otherwise                   = Used
lubUse (UProd {}) (UCall {})       = Used
-- lubUse (UProd {}) Used             = Used
lubUse (UProd ux) Used             = UProd (map (`lubMaybeUsed` useTop) ux)
lubUse Used       (UProd ux)       = UProd (map (`lubMaybeUsed` useTop) ux)
[...]
lubUse Used _                      = Used  -- Note [Used should win]

Note that UProd lub’ed with Used yields UProd. This contradicts the note Used should win. Time for some git archeology... hmm, seems to be like this since dawn of time^Wthe demand analyzer. And as long as no GADTs are around, it probably does not cause problems.

A simple fix would be to change that equation, but surely there are reasons for it...

comment:7 Changed 5 months ago by nomeata

This might be related to Note [Don't optimise UProd(Used) to Used]. There it says

But with the joint demand of <Str, Used> doesn't convey any clue
that there is a product involved, and so the worthSplittingFun
will not fire. (We'd need to use the type as well to make it fire.)

so all this trouble of non-canonical demands just to give some bit of information to worthSplittingFun, which can probably get this information from tryWW, which knows whether the type really is a product type (and not a type variable that was a product type in one branch of a case of the body)? Or are there likely other reasons to keep UProd(Used) instead of UProd?

comment:8 Changed 5 months ago by Joachim Breitner <mail@…>

comment:9 Changed 5 months ago by simonpj

In 7baefa874d858ec1dd8f875a46eb4be5a732875e/testsuite:

(The changeset message doesn't reference this ticket)

comment:10 Changed 3 months ago by Joachim Breitner <breitner@…>

In 61395b59191284a2cc249e614bbba993c16ef8c5/ghc:

type-rep is only broken when debugging is on

in which case it is a wontfix (see #8569)

comment:11 Changed 3 months ago by nomeata

  • Resolution set to wontfix
  • Status changed from new to closed

Guess I did not update the ticket after later discussions.

I believe that this is currently a wontfix:

  • The demand analyser is quite untyped, so in the example in comment:3, where the two branches of a case happen to have quite different types (one being a product, and one not) can only be handled on a best effort basis. Therefore, the code needs to cope with situations where a product demand does not have the right number of components for the constructor at hand. That is also the reason why lubStr (SCall _) (SProd _) should not panic.
  • The Note [Don't optimise UProd(Used) to Used] continues to be relevant: We differentiate these semantically equivalent terms to get some insight about how a product is being used.

I believe the test suite is now set up to reflect this, i.e. type-rep will expectedly fail when GHC is compiled with -DDEBUG *and* the way is an optimizing way.

comment:12 Changed 7 weeks ago by simonpj

  • Resolution wontfix deleted
  • Status changed from closed to new

I’m seeing this assertion failure when using the stage1 compiler (built with –DDEBUG) to compile the stage2 compiler.

ghc-stage1.exe: panic! (the 'impossible' happened)
  (GHC version 7.9.20140227 for i386-unknown-mingw32):
          ASSERT failed!
    file compiler\basicTypes\Demand.lhs line 445
    3
    [U]

Turns out that it's another instance of this ticket, but now the bug is now biting GHC itself! However the type-rep test is a much easier version to debug.

comment:13 Changed 7 weeks ago by Simon Peyton Jones <simonpj@…>

In 4b355cd21a190e3d2c2d3a830ba2337d1c442dfe/ghc:

Make the demand on a binder compatible with type (fixes Trac #8569)

Because of GADTs and casts we were getting binders whose
demand annotation was more deeply nested than made sense
for its type.

See Note [Trimming a demand to a type], in Demand.lhs,
which I reproduce here:

   Note [Trimming a demand to a type]
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Consider this:

     f :: a -> Bool
     f x = case ... of
             A g1 -> case (x |> g1) of (p,q) -> ...
             B    -> error "urk"

   where A,B are the constructors of a GADT.  We'll get a U(U,U) demand
   on x from the A branch, but that's a stupid demand for x itself, which
   has type 'a'. Indeed we get ASSERTs going off (notably in
   splitUseProdDmd, Trac #8569).

   Bottom line: we really don't want to have a binder whose demand is more
   deeply-nested than its type.  There are various ways to tackle this.
   When processing (x |> g1), we could "trim" the incoming demand U(U,U)
   to match x's type.  But I'm currently doing so just at the moment when
   we pin a demand on a binder, in DmdAnal.findBndrDmd.

comment:14 Changed 7 weeks ago by Simon Peyton Jones <simonpj@…>

comment:15 Changed 7 weeks ago by simonpj

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to gadt/type-rep

OK I think I've fixed this.

Should we merge the change to 7.8 branch? Perhaps not unless it actually bites someone. The trouble it that "bite" has only shown up so far as an ASSERT failure, and I'm not 100% sure that it won't have some other bad consequence if the assert is omitted and we proceed regardless. But nothing has so far.

So I'll close for now.

Simon

Note: See TracTickets for help on using tickets.