Opened 5 years ago

Closed 18 months ago

#3132 closed bug (fixed)

cgCase of PrimAlts needs care in new codegen

Reported by: int-e Owned by: simonmar
Priority: high Milestone: 7.8.1
Component: Compiler (NCG) Version: 6.11
Keywords: Cc: benl
Operating System: Unknown/Multiple Architecture: x86
Type of failure: None/Unknown Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

The following code,

module Spring where

import Data.Array.Unboxed

type Arr = UArray Int Double
data Spring = Spring !Double !Int !Arr deriving Show

step :: Double -> Spring -> Spring
step h (Spring k sz y) = let
    f arr = listArray (0, 2*sz-1) (velocity ++ accel)
      where
        velocity = [arr ! i | i <- [sz .. 2*sz-1]]
        k'       = k * fromIntegral sz^2
        accel    = [0] ++ [k' * (arr!(i-1) - 2 * arr!i + arr!(i+1))
                           | i <- [1 .. sz-2]]
                   ++ [k' * (arr!(sz-2) - arr!(sz-1))]
    (.*) :: Double -> Arr -> Arr
    a .* b  = listArray (0, 2*sz-1) $ map (a*) (elems b)
    (<+>) :: Arr -> Arr -> Arr
    a <+> b = listArray (0, 2*sz-1) $ zipWith (+) (elems a) (elems b)
    -- order 4 Runge-Kutta
    k1 = h .* f y
    k2 = h .* f (y <+> (0.5 .* k1))
    k3 = h .* f (y <+> (0.5 .* k2))
    k4 = h .* f (y <+> k3)
    y' = y <+> ((1/6) .* (k1 <+> (2 .* (k2 <+> k3)) <+> k4))
  in
    Spring k sz y'

doesn't compile with optimization in ghc-6.11:

# ghc -O -c Bug.hs
/tmp/ghc12296_0/ghc12296_0.s: Assembler messages:

/tmp/ghc12296_0/ghc12296_0.s:2355:0:
     Error: bad register name `%st(-8)'

/tmp/ghc12296_0/ghc12296_0.s:2384:0:
     Error: bad register name `%st(-8)'

/tmp/ghc12296_0/ghc12296_0.s:2563:0:
     Error: bad register name `%st(-8)'

/tmp/ghc12296_0/ghc12296_0.s:2602:0:
     Error: bad register name `%st(-8)'

/tmp/ghc12296_0/ghc12296_0.s:3006:0:
     Error: bad register name `%fake0'

/tmp/ghc12296_0/ghc12296_0.s:3023:0:
     Error: bad register name `%fake0'

It's odd, first it tries to assign a closure pointer to an FPU register:

        movl $r1sa_closure,%fake0

and later it uses register 0 (%eax) as an operand to an FPU operation:

#       gsubl %fake1,%eax,%fake1
        #GSUB-xxxcase1
        ffree %st(7) ; fld %st(-8) ; fsubrp %st(0),%st(2)

Using -fregs-graph didn't help.

Change History (16)

comment:1 Changed 5 years ago by simonmar

  • Difficulty set to Unknown
  • Milestone set to 6.12 branch
  • Priority changed from normal to high

comment:2 Changed 5 years ago by igloo

  • Milestone changed from 6.12 branch to 6.12.1

Also fails on amd64/Linux in the HEAD, with errors like:

/tmp/ghc22358_0/ghc22358_0.s:2168:0:
     Error: suffix or operands invalid for `subsd'

/tmp/ghc22358_0/ghc22358_0.s:2219:0:
     Error: junk `naughty x86_64 register' after expression

/tmp/ghc22358_0/ghc22358_0.s:2328:0:
     Error: suffix or operands invalid for `addsd'

comment:3 Changed 5 years ago by simonmar

  • Cc benl added
  • Owner set to benl

Ben, would you care to look at this one? (if not, please just unassign yourself)

comment:4 Changed 5 years ago by benl

  • Owner changed from benl to nobody

The cmm code for the failing program contains the following:

        if (_c1Wu::I32 >= 1) goto c1Wx;
        _s1Ba::F64 = r1tw_closure;
        _s1Nk::F64 = %MO_F_Sub_W64(D1, _s1Ba::F64);
        _s1Nl::F64 = %MO_F_Mul_W64(F64[Sp + 12], _s1Nk::F64);

Note that r1tw_closure is not a F64.

Here is a test-case that fails -dcmm-lint as well as -dstg-lint when compiled with -O2.

module Spring where
import Data.Array.Unboxed
type Arr 	= UArray Int Double
step :: Double -> Int -> Arr -> Arr
step h sz y	= listArray (0, 0) [] 
ghc-stage1: panic! (the 'impossible' happened)
  (GHC version 6.11.20090514 for i386-unknown-linux):
            *** Stg Lint ErrMsgs: in Stg2Stg ***
    <no location info>:
         [in body of lambda with binders s{v sGe} [lid] 
                       :: ghc-prim:GHC.Prim.State#{(w) tc 32q}
                                                 s{tv awN} [tv]]
        In a function application, function type doesn't match arg types:
        Function type:
            forall s{tv axt} [tv] i{tv axu} [tv].
            (base:GHC.Arr.Ix{tc 2i} i{tv axu} [tv]) =>
            (i{tv axu} [tv], i{tv axu} [tv])
            -> base:GHC.ST.ST{tc r65}
                 s{tv axt} [tv]
                 (array-0.2.0.1:Data.Array.Base.STUArray{tc r6}
                    s{tv axt} [tv] i{tv axu} [tv] 
                      ghc-prim:GHC.Types.Double{(w) tc 3u})
        Arg types:
            <pred>base:GHC.Arr.Ix{tc 2i} ghc-prim:GHC.Types.Int{(w) tc 3J}
            (ghc-prim:GHC.Types.Int{(w) tc 3J},
             ghc-prim:GHC.Types.Int{(w) tc 3J})
            ghc-prim:GHC.Prim.State#{(w) tc 32q} s{tv awN} [tv]
        Expression:
            array-0.2.0.1:Data.Array.Base.newArray_8{v ra} [gid]
                base:GHC.Arr.$f14{v r9} [gid]
                main:Spring.lvl1{v r8} [gid]
                s{v sGe} [lid]

I'm not sure how to read the STG code, but it looks like something in the libs has been messed up, which has then been inlined.

Perhaps validate should be compiling with all the lint options turned on?

comment:5 Changed 5 years ago by simonmar

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

Fixed:

Wed Jun 17 14:02:27 BST 2009  Simon Marlow <marlowsd@gmail.com>
  * Fix #3132: a case of bogus code generation

comment:6 Changed 5 years ago by simonmar

  • Resolution fixed deleted
  • Status changed from closed to reopened

Ah, we need to leave this ticket open, as a reminder to investigate the issue in the new code generator. See comments in codeGen/CgCase.lhs, search for 3132. The test case is in codeGen/should_compile/3132.hs.

comment:7 Changed 5 years ago by igloo

  • Owner nobody deleted
  • Status changed from reopened to new

comment:8 Changed 5 years ago by simonmar

  • Milestone changed from 6.12.1 to 6.14.1
  • Summary changed from x86 code generator generates bad FPU register names to cgCase of PrimAlts needs care in new codegen

Bumped - no new codegen in 6.12.

comment:9 Changed 5 years ago by simonmar

See also #3286

comment:10 Changed 4 years ago by igloo

  • Blocked By 4258 added
  • Milestone changed from 6.14.1 to 6.16.1

comment:11 Changed 3 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1

comment:12 Changed 22 months ago by simonmar

  • Owner set to simonmar

comment:13 Changed 20 months ago by igloo

  • Milestone changed from 7.6.1 to 7.8.1

comment:14 Changed 18 months ago by michalt

I haven't looked into this ticket (just found it by accident), but isn't it fixed? Test 3132 is enabled by default and compiles just fine with current master...

comment:15 Changed 18 months ago by simonmar

  • Blocked By 4258 removed

comment:16 Changed 18 months ago by simonmar

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

Thanks - yes, I think we can close this. The new codegen has the appropriate special-case code and the comments to go with it, and the test passes.

Note: See TracTickets for help on using tickets.