Profiling broken

The cardinality analysis patch broke profiling. This can be seen in the nightly builds, where the profasm and profthreaded ways show many, many failures. ghc:changeset:da4ff650 is good, and ghc:changeset:62653122 is bad; the intervening few changes are not buildable.

Trac metadata

Trac field	Value
Version	7.7
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC
Operating system
Architecture

changed weight to 5

added Tbug Trac import labels

mentioned in commit bdc3775a

I confirmed that this works on Linux. Thanks!

On latest head, it seems a profiled GHC still segfaults on stg_sel_1_noupd_info. How long until the full fix?

There are also a multitude of profasm/prof/profthreaded tests which are still failing. Here is a partial list:

   codeGen/should_run                   cgrun057 [bad stderr] (profasm,profthreaded)
   concurrent/should_run                T367 [bad exit code] (profasm,ghci,profthreaded)
   indexed-types/should_compile         T4492 [exit code non-0] (profasm)
   indexed-types/should_compile         T7837 [stderr mismatch] (profasm)
   profiling/should_run                 T680 [bad profile] (profasm,profthreaded,prof)
   profiling/should_run                 callstack001 [bad stdout] (prof)
   profiling/should_run                 prof-doc-last [bad profile] (profasm,profthreaded,prof)
   quasiquotation/qq007                 qq007 [exit code non-0] (profasm)
   quasiquotation/qq008                 qq008 [exit code non-0] (profasm)
   simplCore/should_compile             T5550 [exit code non-0] (profasm)
   simplCore/should_compile             T7702 [exit code non-0] (profasm)

changed milestone to %7.8.1

changed weight to 10

release blocker.

Trac metadata

Trac field	Value
Priority	normal → highest

ezyang, can you please paste an example of a test failing due to a segfault?

mentioned in commit 76d0cbcb

I believe the remaining prof failures existed before the cardinality patch. The fix I just checked in should re-instate cost centres in non-top-level closures with no free variables.

If you have a segfaulting test case, please let us know!

Looks like GHC itself is broken when built with BuildFlavour=prof:

===--- building phase 0
make -r --no-print-directory -f ghc.mk phase=0 phase_0_builds
make[1]: Nothing to be done for `phase_0_builds'.
===--- building phase 1
make -r --no-print-directory -f ghc.mk phase=1 phase_1_builds
make[1]: Nothing to be done for `phase_1_builds'.
===--- building final phase
make -r --no-print-directory -f ghc.mk phase=final all
"inplace/bin/ghc-stage2" -hisuf hi -osuf  o -hcsuf hc -static  -H64m -O0 -fasm    -package-name old-time-1.1.0.1 -hide-all-packages -i -ilibraries/old-time/. -ilibraries/old-time/dist-install/build -ilibraries/old-time/dist-install/build/autogen -Ilibraries/old-time/dist-install/build -Ilibraries/old-time/dist-install/build/autogen -Ilibraries/old-time/include    -optP-include -optPlibraries/old-time/dist-install/build/autogen/cabal_macros.h -package base-4.7.0.0 -package old-locale-1.0.0.5 -XHaskell98 -XCPP -XForeignFunctionInterface -O -fasm  -no-user-package-db -rtsopts      -odir libraries/old-time/dist-install/build -hidir libraries/old-time/dist-install/build -stubdir libraries/old-time/dist-install/build  -dynamic-too -c libraries/old-time/dist-install/build/System/Time.hs -o libraries/old-time/dist-install/build/System/Time.o -dyno libraries/old-time/dist-install/build/System/Time.dyn_o
Segmentation fault
make[1]: *** [libraries/old-time/dist-install/build/System/Time.o] Error 139
make: *** [all] Error 2

I just ran into the same problem as parcs, in trying to fix a performance bug it seems I have introduced (in my own branch). Is there a page with advice on fixing performance problems other than Debugging/ProfilingGhc?

I've verified that cardinality + build fixes + mainland workaround result in a segfaulting ghc-stage2, e.g. my log looks like:

375cd37 Add a work-around for #7978.
6265312 Wibbles (merg-os) to ticky-ticky
507c897 Comments about the Name Cache
99d4e5b Implement cardinality analysis

I've also verified that profiling is fine before the cardinality analysis patch.

My guess is cardinality analysis is tickling a latent bug in the code generator related to generating code for non-updatable thunks. If you disable cardinality analysis in the following way:

diff --git a/compiler/stgSyn/CoreToStg.lhs b/compiler/stgSyn/CoreToStg.lhs
index 1395c22..414a6d8 100644
--- a/compiler/stgSyn/CoreToStg.lhs
+++ b/compiler/stgSyn/CoreToStg.lhs
@@ -807,7 +807,7 @@ mkStgRhs rhs_fvs srt bndr binder_info rhs
                   (getFVs rhs_fvs)
                   upd_flag srt [] rhs
   where
-   upd_flag = getUpdateFlag bndr
+   upd_flag = Updatable
   {-
     SDM: disabled.  Eval/Apply can't handle functions with arity zero very
     well; and making these into simple non-updatable thunks breaks other

I think the profiling build goes through as desired.

I've further narrowed down the bug to how selectors are compiled:

--- a/compiler/codeGen/StgCmmBind.hs
+++ b/compiler/codeGen/StgCmmBind.hs
@@ -282,7 +282,7 @@ mkRhsClosure    dflags bndr _cc _bi
     cgRhsStdThunk bndr lf_info [StgVarArg the_fv]
   where
     lf_info               = mkSelectorLFInfo bndr offset_into_int
-                                 (isUpdatable upd_flag)
+                                 (isUpdatable Updatable)
     (_, _, params_w_offsets) = mkVirtConstrOffsets dflags (addIdReps params)
                                -- Just want the layout
     maybe_offset          = assocMaybe params_w_offsets (NonVoid selectee)

Edward: thanks for some great sleuthing. I think that points the finger at the RTS code for non-updatable selector thunks, which has probably never been tested before. I'll try to get around to looking at this, but feel free to dive in first.

mentioned in commit a27895b3