Re: [GHC] #15300: Unboxed Sums Crash

28 Jul 2018

      #15300: Unboxed Sums Crash
-------------------------------------+-------------------------------------
        Reporter:  andrewthad        |                Owner:  osa1
            Type:  bug               |               Status:  merge
        Priority:  highest           |            Milestone:  8.6.1
       Component:  Compiler          |              Version:  8.5
      Resolution:                    |             Keywords:  UnboxedSums
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D4962
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by Ben Gamari <ben@…>):

 In [changeset:"3c311e50e760c3ba00dc9692ad1536c79820598d/ghc"
 3c311e50/ghc]:
 {{{
 #!CommitTicketReference repository="ghc"
 revision="3c311e50e760c3ba00dc9692ad1536c79820598d"
 Run StgCse after unarise, fixes #15300

 Given two unboxed sum terms:

     (# 1 | #) :: (# Int | Int# #)
     (# 1 | #) :: (# Int | Int  #)

 These two terms are not equal as they unarise to different unboxed
 tuples. However StgCse was thinking that these are equal, and replacing
 one of these with a binder to the other.

 To not deal with unboxed sums in StgCse we now do it after unarise. For
 StgCse to maintain post-unarise invariants we factor-out case binder
 in-scopeness check to `stgCaseBndrInScope` and use it in StgCse.

 Also did some refactoring in SimplStg.

 Another way to fix this would be adding a special case in StgCse to not
 bring unboxed sum binders in scope:

     diff --git a/compiler/simplStg/StgCse.hs
 b/compiler/simplStg/StgCse.hs
     index 6c740ca4cb..93a0f8f6ad 100644
     --- a/compiler/simplStg/StgCse.hs
     +++ b/compiler/simplStg/StgCse.hs
     @@ -332,7 +332,11 @@ stgCseExpr env (StgLetNoEscape binds body)
      stgCseAlt :: CseEnv -> OutId -> InStgAlt -> OutStgAlt
      stgCseAlt env case_bndr (DataAlt dataCon, args, rhs)
          = let (env1, args') = substBndrs env args
     -          env2 = addDataCon case_bndr dataCon (map StgVarArg
 args') env1
     +          env2
     +            | isUnboxedSumCon dataCon
     +            = env1
     +            | otherwise
     +            = addDataCon case_bndr dataCon (map StgVarArg args')
 env1
                  -- see note [Case 2: CSEing case binders]
                rhs' = stgCseExpr env2 rhs
            in (DataAlt dataCon, args', rhs')

 I think this patch seems better in that it doesn't add a special case to
 StgCse.

 Test Plan:
 Validate.

 I tried to come up with a minimal example but failed. I thought a simple
 program like

     data T = T (# Int | Int #) (# Int# | Int #)

     case T (# 1 | #) (# 1 | #) of ...

 should be enough to trigger this bug, but for some reason StgCse
 doesn't do
 anything on this program.

 Reviewers: simonpj, bgamari

 Reviewed By: simonpj

 Subscribers: rwbarton, thomie, carter

 GHC Trac Issues: #15300

 Differential Revision: https://phabricator.haskell.org/D4962
 }}}

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/15300#comment:23>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler