Opened 10 years ago

Closed 9 years ago

#3717 closed bug (fixed)

Superfluous seq no eliminated

Reported by: rl Owned by:
Priority: normal Milestone: 7.0.2
Component: Compiler Version: 6.13
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case: simplCore/should_compile/T3717
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Program:

foo :: Int -> Int
foo 0 = 0
foo n = (if n < 5 then 1 else 2) `seq` foo (n-1)

GHC generates this code:

T.$wfoo =
  \ (ww_slm :: GHC.Prim.Int#) ->
    case ww_slm of ds_XkE {
      __DEFAULT ->
        case case GHC.Prim.<# ds_XkE 5 of _ {
               GHC.Bool.False -> lvl1_rlG; GHC.Bool.True -> lvl_rlE
             }
        of _ { __DEFAULT ->
        T.$wfoo (GHC.Prim.-# ds_XkE 1)
        };
      0 -> 0
    }

Note that the seq is still there although it is a nop. This actually occurs in stream fusion code.

Change History (5)

comment:1 Changed 10 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: simplCore/should_compile/T3717

Fixed by

Tue Dec 15 08:01:24 PST 2009  simonpj@microsoft.com
  * Fix Trac #3717: exprOkForSpeculation should look through case expressions
  
  See Note [exprOkForSpeculation: case expressions] in CoreUtils

    M ./compiler/coreSyn/CoreUtils.lhs +35

Simon

comment:2 Changed 9 years ago by rl

patch: 0
Resolution: fixed
Status: closednew

While the patch fixes the original example, it doesn't help here:

foo :: Int -> Int -> Int -> Int
foo x y n = compare x y `seq` n

This generates:

T.foo =
  \ (x_aai :: GHC.Types.Int)
    (y_aaj :: GHC.Types.Int)
    (n_aak :: GHC.Types.Int) ->
    case x_aai of _ { GHC.Types.I# x#_afF [Dmd=Just L] ->
    case y_aaj of _ { GHC.Types.I# y#_afJ [Dmd=Just L] ->
    case case GHC.Prim.<# x#_afF y#_afJ of _ {
           GHC.Bool.False ->
             case GHC.Prim.==# x#_afF y#_afJ of _ {
               GHC.Bool.False -> GHC.Ordering.GT; GHC.Bool.True -> GHC.Ordering.EQ
             };
           GHC.Bool.True -> GHC.Ordering.LT
         }
    of _ { __DEFAULT ->
    n_aak
    }
    }
    }

comment:3 Changed 9 years ago by igloo

Milestone: 6.14.1

comment:4 Changed 9 years ago by igloo

Milestone: 7.0.17.0.2

comment:5 Changed 9 years ago by simonpj

Resolution: fixed
Status: newclosed

Fixed by

Tue Jan 25 11:05:25 GMT 2011  simonpj@microsoft.com
  * Fix Trac #3717 by making exprOkForSpeculation a bit cleverer
  
  The main change here is to do with dropping redundant seqs.
  See Note [exprOkForSpeculation: case expressions] in CoreUtils.

    M ./compiler/coreSyn/CoreUtils.lhs -5 +14
    M ./compiler/simplCore/Simplify.lhs -36 +47

Tue Jan 25 11:04:18 GMT 2011  simonpj@microsoft.com
  * Improve dataToTag# magic
  
  dataToTag# is a bit unsatisfactory because it requires
  its argument to be evaluated, and we don't have a good
  way to enforce that. This patch adds some comments, and
  makes exprOkForSpeculation a bit less picky in the case
  of dataToTag# (since the argument may, in fact, not be
  eval'd).

    M ./compiler/coreSyn/CorePrep.lhs -3 +8
    M ./compiler/coreSyn/CoreUtils.lhs +24

No need to merge; putting this in 7.2 is fine.

Simon

Note: See TracTickets for help on using tickets.