Opened 2 years ago

Last modified 17 months ago

#5780 new bug

-faggressive-primops change caused a failure in perf/compiler/parsing001

Reported by: simonmar Owned by:
Priority: high Milestone: 7.8.3
Component: Compiler Version: 7.2.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

The commit 601c983dd0bada6b49bdadd8f172fd4eacac4b0c apparently caused a regression in perf/compiler/parsing001, so I'm reverting it pending investigation. The patch should have made no changes to performance, because the new functionality was only enabled by a new flag.

Change History (4)

comment:1 Changed 2 years ago by marlowsd@…

commit 395ef284545bb5e7a57c180cc1f2a1b66fa30f37

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Mon Jan 16 12:41:56 2012 +0000

    Revert "Add -faggressive-primops plus refactoring in CoreUtils" (#5780)
    
    This reverts commit 601c983dd0bada6b49bdadd8f172fd4eacac4b0c.

 compiler/coreSyn/CoreArity.lhs    |    4 +-
 compiler/coreSyn/CoreUtils.lhs    |  237 ++++++++++++++++--------------------
 compiler/main/StaticFlagParser.hs |    1 -
 compiler/main/StaticFlags.hs      |    6 -
 compiler/prelude/PrimOp.lhs       |   59 ++-------
 compiler/simplCore/FloatIn.lhs    |   10 +--
 compiler/simplCore/OccurAnal.lhs  |    4 +-
 compiler/simplCore/SimplUtils.lhs |   10 +-
 compiler/simplCore/Simplify.lhs   |   20 ++--
 9 files changed, 137 insertions(+), 214 deletions(-)

comment:2 Changed 2 years ago by simonpj

BTW here's the original commit message (the one that was reverted):

commit 601c983dd0bada6b49bdadd8f172fd4eacac4b0c
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Fri Jan 13 17:50:00 2012 +0000

    Add -faggressive-primops plus refactoring in CoreUtils
    
    I'm experimenting with making GHC a bit more aggressive about
      a) dropping case expressions if the result is unused
            Simplify.rebuildCase, CaseElim equation
    
      b) floating case expressions inwards
            FloatIn.fiExpr, AnnCase
    
    In both cases the new behaviour is gotten with a static (debug)
    flag -faggressive-primops.  The extra "aggression" is to allow
    discarding and floating in for side-effecting operations.  See
    the new, extensive Note [PrimOp can_fail and has_side_effects]
    in PrimoOp.
    
    When discarding a case with unused binders, in the lifted-type
    case it's definitely ok if the scrutinee terminates; previously
    we were checking exprOkForSpeculation, which is significantly
    worse.
    
    So I wanted a new function CoreUtils.exprCertainlyTerminates.
    In doing this I ended up with a significant refactoring in
    CoreUtils.  The new structure has quite a lot of nice sharing:
    
        exprIsCheap             = exprIsCheap' isHNFApp
        exprIsExpandable        = exprIsCheap' isConLikeApp
    
        exprIsHNF               = exprIsHNFlike isHNFApp
        exprIsConLike           = exprIsHNFlike isConLikeApp
        exprCertainlyTerminates = exprIsHNFlike isTerminatingApp

 compiler/coreSyn/CoreArity.lhs    |    4 +-
 compiler/coreSyn/CoreUtils.lhs    |  237 ++++++++++++++++++++----------------
 compiler/main/StaticFlagParser.hs |    1 +
 compiler/main/StaticFlags.hs      |    6 +
 compiler/prelude/PrimOp.lhs       |   59 +++++++--
 compiler/simplCore/FloatIn.lhs    |   10 ++-
 compiler/simplCore/OccurAnal.lhs  |    4 +-
 compiler/simplCore/SimplUtils.lhs |   10 +-
 compiler/simplCore/Simplify.lhs   |   20 ++--
 9 files changed, 214 insertions(+), 137 deletions(-)

comment:3 Changed 20 months ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:4 Changed 17 months ago by igloo

  • Milestone changed from 7.6.2 to 7.8.1
Note: See TracTickets for help on using tickets.