Opened 7 years ago

Closed 3 years ago

#2552 closed bug (fixed)

SCC annotation behavior differs between toplevel and non-toplevel

Reported by: Rauli Owned by: simonmar
Priority: high Milestone: 7.4.1
Component: Profiling Version: 6.8.2
Keywords: scc profiling Cc: ben@…
Operating System: Linux Architecture: x86
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

I'm not sure if this is a bug, but using ghc -prof -auto-all and manual SCC annotations for non-toplevel functions produces very different results from moving the same function to toplevel:

fib n = nfib' n
  where
    nfib' n = {-# SCC "nfib'" #-} nfib n -- %time: 0 (both individual and inherited)
      where
        nfib n = if n < 2 then 1 else nfib (n-1) + nfib (n-2)

fib n = nfib' n
nfib' n = nfib n -- %time: 100 (both individual and inherited)
  where
    nfib n = if n < 2 then 1 else nfib (n-1) + nfib (n-2)

This happens with both -O0 and -O2, even when these functions are not exported. Adding a NOINLINE for nfib' doesn't change anything.

Computing the correct inherited time for the SCC would be useful to pinpoint the cost centres inside a more complex function. Currently one needs to move them to toplevel to get the same effect. Given that the toplevel route works, it's also counterintuitive that manual SCC annotations don't.

Attachments (1)

fib.hs (970 bytes) - added by Rauli 7 years ago.
Example code

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Rauli

Example code

comment:1 Changed 7 years ago by igloo

  • difficulty set to Unknown
  • Milestone set to 6.10 branch

comment:2 Changed 6 years ago by BenMoseley

  • Cc ben@… added

comment:3 Changed 6 years ago by igloo

  • Milestone changed from 6.10 branch to 6.12 branch

comment:4 Changed 5 years ago by igloo

  • Milestone changed from 6.12 branch to 6.12.3

comment:5 Changed 5 years ago by igloo

  • Milestone changed from 6.12.3 to 6.14.1
  • Priority changed from normal to low

comment:6 Changed 4 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:7 Changed 4 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:8 Changed 4 years ago by simonmar

  • Component changed from Compiler to Profiling
  • Milestone changed from 7.2.1 to 7.4.1
  • Owner set to simonmar
  • Priority changed from low to high
  • Type of failure set to None/Unknown

comment:9 Changed 3 years ago by marlowsd@…

commit 7bb0447df9a783c222c2a077e35e5013c7c68d91

Author: Simon Marlow <[email protected]>
Date:   Thu Oct 27 13:47:27 2011 +0100

    Overhaul of infrastructure for profiling, coverage (HPC) and breakpoints
    
    User visible changes
    ====================
    
    Profilng
    --------
    
    Flags renamed (the old ones are still accepted for now):
    
      OLD            NEW
      ---------      ------------
      -auto-all      -fprof-auto
      -auto          -fprof-exported
      -caf-all       -fprof-cafs
    
    New flags:
    
      -fprof-auto              Annotates all bindings (not just top-level
                               ones) with SCCs
    
      -fprof-top               Annotates just top-level bindings with SCCs
    
      -fprof-exported          Annotates just exported bindings with SCCs
    
      -fprof-no-count-entries  Do not maintain entry counts when profiling
                               (can make profiled code go faster; useful with
                               heap profiling where entry counts are not used)
    
    Cost-centre stacks have a new semantics, which should in most cases
    result in more useful and intuitive profiles.  If you find this not to
    be the case, please let me know.  This is the area where I have been
    experimenting most, and the current solution is probably not the
    final version, however it does address all the outstanding bugs and
    seems to be better than GHC 7.2.
    
    Stack traces
    ------------
    
    +RTS -xc now gives more information.  If the exception originates from
    a CAF (as is common, because GHC tends to lift exceptions out to the
    top-level), then the RTS walks up the stack and reports the stack in
    the enclosing update frame(s).
    
    Result: +RTS -xc is much more useful now - but you still have to
    compile for profiling to get it.  I've played around a little with
    adding 'head []' to GHC itself, and +RTS -xc does pinpoint the problem
    quite accurately.
    
    I plan to add more facilities for stack tracing (e.g. in GHCi) in the
    future.
    
    Coverage (HPC)
    --------------
    
     * derived instances are now coloured yellow if they weren't used
     * likewise record field names
     * entry counts are more accurate (hpc --fun-entry-count)
     * tab width is now correct (markup was previously off in source with
       tabs)
    
    Internal changes
    ================
    
    In Core, the Note constructor has been replaced by
    
            Tick (Tickish b) (Expr b)
    
    which is used to represent all the kinds of source annotation we
    support: profiling SCCs, HPC ticks, and GHCi breakpoints.
    
    Depending on the properties of the Tickish, different transformations
    apply to Tick.  See CoreUtils.mkTick for details.
    
    Tickets
    =======
    
    This commit closes the following tickets, test cases to follow:
    
      - Close #2552: not a bug, but the behaviour is now more intuitive
        (test is T2552)
    
      - Close #680 (test is T680)
    
      - Close #1531 (test is result001)
    
      - Close #949 (test is T949)
    
      - Close #2466: test case has bitrotted (doesn't compile against current
        version of vector-space package)

 compiler/basicTypes/Id.lhs           |   17 +-
 compiler/basicTypes/MkId.lhs         |   28 +-
 compiler/basicTypes/Name.lhs         |   20 +-
 compiler/cmm/CmmParse.y              |    1 -
 compiler/codeGen/CgCallConv.hs       |   15 +-
 compiler/codeGen/CgClosure.lhs       |   31 +-
 compiler/codeGen/CgCon.lhs           |    6 +-
 compiler/codeGen/CgExpr.lhs          |    2 +-
 compiler/codeGen/CgHeapery.lhs       |    8 +-
 compiler/codeGen/CgProf.hs           |  195 +-----
 compiler/codeGen/StgCmmBind.hs       |   23 +-
 compiler/codeGen/StgCmmCon.hs        |    4 +-
 compiler/codeGen/StgCmmExpr.hs       |    2 +-
 compiler/codeGen/StgCmmHeap.hs       |    5 -
 compiler/codeGen/StgCmmProf.hs       |  177 +-----
 compiler/coreSyn/CoreArity.lhs       |   20 +-
 compiler/coreSyn/CoreFVs.lhs         |   14 +-
 compiler/coreSyn/CoreLint.lhs        |    8 +-
 compiler/coreSyn/CorePrep.lhs        |   56 +-
 compiler/coreSyn/CoreSubst.lhs       |   35 +-
 compiler/coreSyn/CoreSyn.lhs         |  123 +++-
 compiler/coreSyn/CoreTidy.lhs        |    9 +-
 compiler/coreSyn/CoreUnfold.lhs      |    6 +-
 compiler/coreSyn/CoreUtils.lhs       |  187 ++++--
 compiler/coreSyn/ExternalCore.lhs    |    2 +-
 compiler/coreSyn/MkExternalCore.lhs  |    3 +-
 compiler/coreSyn/PprCore.lhs         |   35 +-
 compiler/coreSyn/PprExternalCore.lhs |    2 +-
 compiler/coreSyn/TrieMap.lhs         |   25 +-
 compiler/deSugar/Coverage.lhs        |  556 +++++++++++------
 compiler/deSugar/Desugar.lhs         |   35 +-
 compiler/deSugar/DsArrows.lhs        |    5 +-
 compiler/deSugar/DsBinds.lhs         |  146 ++---
 compiler/deSugar/DsExpr.lhs          |   13 +-
 compiler/deSugar/DsGRHSs.lhs         |   11 +-
 compiler/deSugar/DsUtils.lhs         |   71 +--
 compiler/deSugar/Match.lhs           |    2 +-
 compiler/ghci/ByteCodeGen.lhs        |  127 ++---
 compiler/hsSyn/Convert.lhs           |    3 +-
 compiler/hsSyn/HsBinds.lhs           |   15 +-
 compiler/hsSyn/HsExpr.lhs            |   14 +-
 compiler/iface/BinIface.hs           |   64 +--
 compiler/iface/IfaceEnv.lhs          |   18 -
 compiler/iface/IfaceSyn.lhs          |   33 +-
 compiler/iface/MkIface.lhs           |   12 +-
 compiler/iface/TcIface.lhs           |   15 +-
 compiler/main/DynFlags.hs            |   54 +-
 compiler/main/HscMain.hs             |   17 +-
 compiler/main/TidyPgm.lhs            |    5 +-
 compiler/parser/Parser.y.pp          |    2 +-
 compiler/parser/RdrHsSyn.lhs         |    3 +-
 compiler/profiling/CostCentre.lhs    |  407 ++++--------
 compiler/profiling/SCCfinal.lhs      |  285 ++------
 compiler/simplCore/CSE.lhs           |    2 +-
 compiler/simplCore/FloatIn.lhs       |   11 +-
 compiler/simplCore/FloatOut.lhs      |   25 +-
 compiler/simplCore/LiberateCase.lhs  |    2 +-
 compiler/simplCore/OccurAnal.lhs     |   28 +-
 compiler/simplCore/SAT.lhs           |    4 +-
 compiler/simplCore/SetLevels.lhs     |   10 +-
 compiler/simplCore/SimplCore.lhs     |    2 +-
 compiler/simplCore/SimplEnv.lhs      |   33 +-
 compiler/simplCore/SimplUtils.lhs    |   21 +-
 compiler/simplCore/Simplify.lhs      |  122 +++-
 compiler/simplStg/SRT.lhs            |    2 +-
 compiler/simplStg/StgStats.lhs       |    2 +-
 compiler/specialise/Rules.lhs        |   19 +-
 compiler/specialise/SpecConstr.lhs   |   14 +-
 compiler/specialise/Specialise.lhs   |   16 +-
 compiler/stgSyn/CoreToStg.lhs        |   27 +-
 compiler/stgSyn/StgLint.lhs          |    2 +-
 compiler/stgSyn/StgSyn.lhs           |   13 +-
 compiler/stranal/DmdAnal.lhs         |    4 +-
 compiler/stranal/WorkWrap.lhs        |    6 +-
 compiler/stranal/WwLib.lhs           |   14 +-
 compiler/typecheck/TcBinds.lhs       |    3 +-
 compiler/vectorise/Vectorise/Exp.hs  |   15 +-
 compiler/vectorise/Vectorise/Vect.hs |    6 +-
 docs/users_guide/flags.xml           |   47 +-
 docs/users_guide/profiling.xml       |  560 +++++++++-------
 docs/users_guide/runtime_control.xml |   56 ++-
 includes/rts/prof/CCS.h              |  190 +++---
 includes/stg/MiscClosures.h          |    1 +
 rts/Apply.cmm                        |   25 +-
 rts/AutoApply.h                      |   11 +
 rts/Exception.cmm                    |    4 +-
 rts/PrimOps.cmm                      |    4 +
 rts/ProfHeap.c                       |    4 +-
 rts/Profiling.c                      | 1200 ++++++++++++++++++----------------
 rts/Profiling.h                      |    3 +-
 rts/Proftimer.c                      |    2 +
 rts/RaiseAsync.c                     |    2 +-
 rts/RtsFlags.c                       |    7 +-
 rts/StgMiscClosures.cmm              |   16 +-
 rts/Updates.cmm                      |    2 +-
 rts/sm/Storage.c                     |    6 +-
 utils/genapply/GenApply.hs           |   65 ++-
 97 files changed, 2750 insertions(+), 2830 deletions(-)

comment:10 Changed 3 years ago by simonmar

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.