Fix a bug in SRT generation
ClosedPublic

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

Details

Summary

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

Harbourmaster

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
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.run  T9405 [unexpected] (normal)

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

Unexpected stat failures:
   perf/compiler/T6048.run              T6048 [stat not good enough] (optasm)
   perf/compiler/T12234.run             T12234 [stat not good enough] (optasm)
   perf/compiler/T13035.run             T13035 [stat not good enough] (normal)
   perf/compiler/MultiLayerModules.run  MultiLayerModules [stat not good enough] (normal)
   perf/haddock/haddock.base.run        haddock.base [stat not good enough] (normal)
   perf/haddock/haddock.compiler.run    haddock.compiler [stat not good enough] (normal)
   perf/should_run/T14936.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.