Fix a bug in SRT generation

Authored by simonmar on May 22 2018, 9:50 AM.



I had good intentions, but they were not being followed. In particular,
this comment:

---  - we never resolve a reference to a CAF to the contents of its SRT, since
---    the point of SRTs is to keep CAFs alive.

was not true, because we updated the srtMap after generating the SRT
for a CAF. Therefore it was possible for another CAF to refer to an
earlier CAF, and the reference to the earlier CAF would be shortcutted
to refer to its SRT instead of pointing to the CAF itself.

The fix is just to not update the srtMap when generating the SRT for a
CAF, but I also refactored the code and comments around this to be a bit
better organised.

Test Plan


Diff Detail

rGHC Glasgow Haskell Compiler
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
simonmar created this revision.May 22 2018, 9:50 AM

Well, I've been waiting 17h for a build and it looks like it's still queued. If anyone is able to validate on Windows that would be great. cc @awson @Phyx

Phyx added a comment.May 23 2018, 7:23 AM

I have a validate running. Will post the results when it finishes.

Phyx added a comment.May 23 2018, 10:55 AM

Results for the validate

Unexpected results from:
TEST="MultiLayerModules T12234 T13035 T14695 T14936 T6048 T9293 T9405 bkpcabal06 bkpcabal07 derefnull divbyzero ghci024 ghci057 haddock.base haddock.compiler tempfiles"

       0 had missing libraries
       8 expected passes
       0 expected failures

       0 caused framework failures
       0 caused framework warnings
       1 unexpected passes
       9 unexpected failures
       7 unexpected stat failures

Unexpected passes:
   rts/  T9405 [unexpected] (normal)

Unexpected failures:
   backpack/cabal/bkpcabal06/  bkpcabal06 [bad exit code] (normal)
   backpack/cabal/bkpcabal07/  bkpcabal07 [bad exit code] (normal)
   ghci/scripts/                  ghci024 [bad stdout] (normal)
   ghci/scripts/                  ghci057 [bad stdout] (ghci)
   ghci/scripts/                    T9293 [bad stdout] (ghci)
   rts/                         derefnull [bad exit code] (normal)
   rts/                         divbyzero [bad exit code] (normal)
   rts/                            T14695 [bad stderr] (normal)
   ../../libraries/base/tests/  tempfiles [bad stdout] (normal)

Unexpected stat failures:
   perf/compiler/              T6048 [stat not good enough] (optasm)
   perf/compiler/             T12234 [stat not good enough] (optasm)
   perf/compiler/             T13035 [stat not good enough] (normal)
   perf/compiler/  MultiLayerModules [stat not good enough] (normal)
   perf/haddock/        haddock.base [stat not good enough] (normal)
   perf/haddock/    haddock.compiler [stat not good enough] (normal)
   perf/should_run/           T14936 [stat too good] (normal)

These are all normal (I have another diff to clean these up). So the patch passes validate.

@Phyx thanks so much for this! I'll go ahead and push.

This revision was not accepted when it landed; it landed in state Needs Review.May 23 2018, 12:17 PM
This revision was automatically updated to reflect the committed changes.