Read in unfolding with -O0 and check for inline on usage (#9370)
AbandonedPublic

Authored by richardfung on Aug 28 2016, 1:44 PM.

Details

Reviewers
simonmar
austin
bgamari
Trac Issues
#9370
Summary

Make -O0 no longer imply -fignore-interface-pragmas. This should fix
an issue where interface pragmas were leaking between files with
different optimization levels in --make mode

Diff Detail

richardfung updated this revision to Diff 8506.Aug 28 2016, 1:44 PM
richardfung retitled this revision from to Read in unfolding with -O0 and check for inline on usage (#9370).
richardfung updated this object.
richardfung edited the test plan for this revision. (Show Details)
thomie edited edge metadata.Aug 28 2016, 5:57 PM
thomie updated the Trac tickets for this revision.
bgamari requested changes to this revision.Aug 29 2016, 11:42 AM
bgamari edited edge metadata.

Thanks for picking this one up, @richardfung! It's never easy diving into a new project and your persistence is appreciated!

testsuite/tests/simplCore/should_compile/T9370a.hs
4

It would be nice to see a short comment here explaining what in particular we are looking for in the output from this test. Something like,

We should see no inlinings emitted by -ddump-inlinings since we have specified -O0.

This sort of thing is quite handy when someone needs to update the expected test output in the future as otherwise they are left guessing whether the new output is indeed correct.

This revision now requires changes to proceed.Aug 29 2016, 11:42 AM
richardfung edited edge metadata.

Added comment to test case per Ben Gamari's suggestion

Thanks for picking this one up, @richardfung! It's never easy diving into a new project and your persistence is appreciated!

Thank you for all the help! I definitely learned a lot already..

I've added the requested comment.

However, I've noticed that when trying to use "make test" or validate locally, I do have some errors. I actually have errors even when doing either of those things from HEAD in master, although not quite the same ones..

I submitted this request thinking there might be something about my local environment that's causing the issue. Does the fact that this change has been marked as "buildable" imply it's going to be okay? I tried looking on the Phabricator page on the wiki, but it says "See Harbormaster below", except there is no section for Harbormaster

However, I've noticed that when trying to use "make test" or validate locally, I do have some errors. I actually have errors even when doing either of those things from HEAD in master, although not quite the same ones.

Hmm, I'll try to build locally and see if I can reproduce. In the meantime could you paste some of the interesting parts of the validate output here?

However, I've noticed that when trying to use "make test" or validate locally, I do have some errors. I actually have errors even when doing either of those things from HEAD in master, although not quite the same ones.

Hmm, I'll try to build locally and see if I can reproduce. In the meantime could you paste some of the interesting parts of the validate output here?

Here is the testsuite_summary from running validate on HEAD:

Unexpected results from:
TEST="T12355 T12531 derefnull divbyzero T6132"

SUMMARY for test run started at Sun Aug 28 22:18:24 2016 PDT
 0:32:58 spent to go through
    5317 total tests, which gave rise to
   17018 test cases, of which
   11439 were skipped

      51 had missing libraries
    5399 expected passes
     124 expected failures

       0 caused framework failures
       1 unexpected passes
       4 unexpected failures
       0 unexpected stat failures

Unexpected passes:
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-mAQOiC/test   spaces/./codeGen/should_compile/T12355.run  T12355 [unexpected] (normal)

Unexpected failures:
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-mAQOiC/test   spaces/./partial-sigs/should_compile/T12531.run  T12531 [stderr mismatch] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-mAQOiC/test   spaces/./rts/derefnull.run                       derefnull [bad stderr] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-mAQOiC/test   spaces/./rts/divbyzero.run                       divbyzero [bad stderr] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-mAQOiC/test   spaces/./runghc/T6132.run                        T6132 [exit code non-0] (normal)

And with my changes:

Unexpected results from:
TEST="T12355 T3064 T5030 break021 divbyzero T4029 T5321FD T6132 T12531 derefnull parsing001 T5321Fun T3294"

SUMMARY for test run started at Mon Aug 29 12:42:48 2016 PDT
 0:27:51 spent to go through
    5318 total tests, which gave rise to
   17021 test cases, of which
   11441 were skipped

      51 had missing libraries
    5392 expected passes
     124 expected failures

       0 caused framework failures
       1 unexpected passes
       5 unexpected failures
       7 unexpected stat failures

Unexpected passes:
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./codeGen/should_compile/T12355.run  T12355 [unexpected] (normal)

Unexpected failures:
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./ghci.debugger/scripts/break021.run      break021 [bad stdout] (ghci)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./partial-sigs/should_compile/T12531.run  T12531 [stderr mismatch] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./rts/divbyzero.run                       divbyzero [bad stderr] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./rts/derefnull.run                       derefnull [bad stderr] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./runghc/T6132.run                        T6132 [exit code non-0] (normal)

Unexpected stat failures:
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/T3064.run       T3064 [stat too good] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/T5030.run       T5030 [stat not good enough] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/parsing001.run  parsing001 [stat not good enough] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/T5321Fun.run    T5321Fun [stat too good] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/T3294.run       T3294 [stat too good] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/compiler/T5321FD.run     T5321FD [stat too good] (normal)
   /var/folders/ct/2d6k574940517x7xyzc44tdr0001jt/T/ghctest-nFivrK/test   spaces/./perf/space_leaks/T4029.run    T4029 [stat not good enough] (ghci)

It does seem like the ones that fail on HEAD do continue to fail with my changes though.. but from what I can tell, validate seems to think my changes are fine on Harbormaster?

I'll have a look shortly.

Hmm, so I'm a bit unclear as to why you are seeing testsuite failures on HEAD without your patch. The tree currently validates for me. What operating system are you working on? Can you paste the output of,

$ make test TEST="T12355 T12531 derefnull divbyzero T6132"
bgamari requested changes to this revision.Sep 1 2016, 10:11 AM
bgamari edited edge metadata.

Also, perhaps try rebasing on master. The tree has been a bit shaky over the last few days.

This revision now requires changes to proceed.Sep 1 2016, 10:11 AM

Hmm, so I'm a bit unclear as to why you are seeing testsuite failures on HEAD without your patch. The tree currently validates for me. What operating system are you working on? Can you paste the output of,

$ make test TEST="T12355 T12531 derefnull divbyzero T6132"

I'm on OS X, although I have tested it on Arch 64 bit and it also has failing tests.. I'm not sure if they are the exact same ones though.

This is what that command gives me, after updating to the latest on master

=====> T12355(normal) 1 of 5 [0, 0, 0]
cd "./codeGen/should_compile/T12355.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T12355.hs -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  
*** unexpected pass for T12355(normal)
=====> T12531(normal) 2 of 5 [1, 0, 0]
cd "./partial-sigs/should_compile/T12531.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T12531.hs -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  -fdefer-typed-holes
=====> derefnull(normal) 3 of 5 [1, 0, 0]
cd "./rts/derefnull.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -o derefnull derefnull.hs -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  
cd "./rts/derefnull.run" && ./derefnull  
Actual stderr output differs from expected:
--- /dev/null	2016-09-01 15:51:58.000000000 -0700
+++ ./rts/derefnull.run/derefnull.run.stderr.normalised	2016-09-01 15:52:02.000000000 -0700
@@ -0,0 +1 @@
+/bin/sh: line 1: 51069 Segmentation fault: 11  ./derefnull
*** unexpected failure for derefnull(normal)
=====> divbyzero(normal) 4 of 5 [1, 1, 0]
cd "./rts/divbyzero.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -o divbyzero divbyzero.hs -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  
cd "./rts/divbyzero.run" && ./divbyzero  
Actual stderr output differs from expected:
--- /dev/null	2016-09-01 15:51:58.000000000 -0700
+++ ./rts/divbyzero.run/divbyzero.run.stderr.normalised	2016-09-01 15:52:03.000000000 -0700
@@ -0,0 +1 @@
+/bin/sh: line 1: 51086 Floating point exception: 8   ./divbyzero
*** unexpected failure for divbyzero(normal)
=====> T6132(normal) 5 of 5 [1, 2, 0]
cd "./runghc/T6132.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T6132.hs -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  
Compile failed (exit code 1) errors were:

T6132.hs:1:2: error: parse error on input ‘#!/’

*** unexpected failure for T6132(normal)

Unexpected results from:
TEST="T12355 derefnull divbyzero T6132"

SUMMARY for test run started at Thu Sep  1 15:51:59 2016 PDT
 0:00:05 spent to go through
       5 total tests, which gave rise to
      23 test cases, of which
      18 were skipped

       0 had missing libraries
       1 expected passes
       0 expected failures

       0 caused framework failures
       1 unexpected passes
       3 unexpected failures
       0 unexpected stat failures

Unexpected passes:
   codeGen/should_compile/T12355.run  T12355 [unexpected] (normal)

Unexpected failures:
   rts/derefnull.run  derefnull [bad stderr] (normal)
   rts/divbyzero.run  divbyzero [bad stderr] (normal)
   runghc/T6132.run   T6132 [exit code non-0] (normal)

I took a look at this today on the GHC OS X box and found similar crashes on master in both derefnull and divbyzero. Sadly I have no idea when this regressed.

Okay I retested head earlier on arch and it seems to run fine so I think this is just an OS X issue. Should I make new tickets to fix these tests?

I took a look at this today on the GHC OS X box and found similar crashes on master in both derefnull and divbyzero. Sadly I have no idea when this regressed.

I would be happy to help find when this happened, but for now I'd like to get this in =D

My patch unfortunately still has some errors in validate besides the ones which are already broken on OS X.

These are the ones which fail:
TEST="T12355 T5321FD parsing001 break021 divbyzero derefnull T6132 T3064 T5030 T5321Fun T3294"

The output of make test with those tests is

=====> break021(ghci) 1 of 7 [0, 0, 0]
cd "./ghci.debugger/scripts/break021.run" && HC="/Users/richard/ghc/inplace/test   spaces/ghc-stage2" HC_OPTS="-dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups " "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  --interactive -v0 -ignore-dot-ghci -fno-ghci-history +RTS -I0.1 -RTS  -ignore-dot-ghci< break021.script
Actual stdout output differs from expected:
--- ./ghci.debugger/scripts/break021.run/break021.stdout.normalised	2016-09-08 22:48:48.000000000 -0700
+++ ./ghci.debugger/scripts/break021.run/break021.run.stdout.normalised	2016-09-08 22:48:48.000000000 -0700
@@ -17,7 +17,7 @@
       ^^^^^^^
 11    line2 0
 Stopped in Main.line1, break020.hs:3:11-19
-_result :: IO () = _
+_result :: m () = _
 2  
 3  line1 _ = return ()
              ^^^^^^^^^
@@ -29,7 +29,7 @@
       ^^^^^^^
 12    in_another_decl 0
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
@@ -55,7 +55,7 @@
                           ^^^^^^^
 7                         line2 0
 Stopped in Main.line1, break020.hs:3:11-19
-_result :: IO () = _
+_result :: m () = _
 2  
 3  line1 _ = return ()
              ^^^^^^^^^
@@ -67,7 +67,7 @@
                           ^^^^^^^
 8  
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
@@ -85,7 +85,7 @@
       ^^^^^^^
 15    return ()
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
*** unexpected failure for break021(ghci)
=====> T3294(normal) 2 of 7 [0, 1, 0]
cd "./perf/compiler/T3294.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T3294.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups +RTS -G1 -RTS  +RTS -V0 -tT3294.comp.stats --machine-readable -RTS
bytes allocated value is too low:
(If this is because you have improved GHC, please
update the test so that GHC doesn't regress again)
    Expected    T3294(normal) bytes allocated: 2739731144 +/-5%
    Lower bound T3294(normal) bytes allocated: 2602744586 
    Upper bound T3294(normal) bytes allocated: 2876717702 
    Actual      T3294(normal) bytes allocated:  848278376 
    Deviation   T3294(normal) bytes allocated:      -69.0 %
*** unexpected stat test failure for T3294(normal)
=====> T3064(normal) 3 of 7 [0, 1, 0]
cd "./perf/compiler/T3064.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T3064.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups +RTS -G1 -RTS  +RTS -V0 -tT3064.comp.stats --machine-readable -RTS
bytes allocated value is too low:
(If this is because you have improved GHC, please
update the test so that GHC doesn't regress again)
    Expected    T3064(normal) bytes allocated: 287460128 +/-5%
    Lower bound T3064(normal) bytes allocated: 273087121 
    Upper bound T3064(normal) bytes allocated: 301833135 
    Actual      T3064(normal) bytes allocated: 259294592 
    Deviation   T3064(normal) bytes allocated:      -9.8 %
*** unexpected stat test failure for T3064(normal)
=====> T5030(normal) 4 of 7 [0, 1, 0]
cd "./perf/compiler/T5030.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T5030.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  -freduction-depth=300 +RTS -V0 -tT5030.comp.stats --machine-readable -RTS
bytes allocated value is too high:
    Expected    T5030(normal) bytes allocated: 653710960 +/-10%
    Lower bound T5030(normal) bytes allocated: 588339864 
    Upper bound T5030(normal) bytes allocated: 719082056 
    Actual      T5030(normal) bytes allocated: 820392784 
    Deviation   T5030(normal) bytes allocated:      25.5 %
*** unexpected stat test failure for T5030(normal)
=====> parsing001(normal) 5 of 7 [0, 1, 0]
cd "./perf/compiler/parsing001.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c parsing001.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups   +RTS -V0 -tparsing001.comp.stats --machine-readable -RTS
bytes allocated value is too high:
    Expected    parsing001(normal) bytes allocated: 581551384 +/-5%
    Lower bound parsing001(normal) bytes allocated: 552473814 
    Upper bound parsing001(normal) bytes allocated: 610628954 
    Actual      parsing001(normal) bytes allocated: 687926504 
    Deviation   parsing001(normal) bytes allocated:      18.3 %
*** unexpected stat test failure for parsing001(normal)
=====> T5321Fun(normal) 6 of 7 [0, 1, 0]
cd "./perf/compiler/T5321Fun.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T5321Fun.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups   +RTS -V0 -tT5321Fun.comp.stats --machine-readable -RTS
bytes allocated value is too low:
(If this is because you have improved GHC, please
update the test so that GHC doesn't regress again)
    Expected    T5321Fun(normal) bytes allocated: 565883176 +/-10%
    Lower bound T5321Fun(normal) bytes allocated: 509294858 
    Upper bound T5321Fun(normal) bytes allocated: 622471494 
    Actual      T5321Fun(normal) bytes allocated: 284549200 
    Deviation   T5321Fun(normal) bytes allocated:     -49.7 %
*** unexpected stat test failure for T5321Fun(normal)
=====> T5321FD(normal) 7 of 7 [0, 1, 0]
cd "./perf/compiler/T5321FD.run" &&  "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -c T5321FD.hs -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups   +RTS -V0 -tT5321FD.comp.stats --machine-readable -RTS
bytes allocated value is too low:
(If this is because you have improved GHC, please
update the test so that GHC doesn't regress again)
    Expected    T5321FD(normal) bytes allocated: 477840432 +/-10%
    Lower bound T5321FD(normal) bytes allocated: 430056388 
    Upper bound T5321FD(normal) bytes allocated: 525624476 
    Actual      T5321FD(normal) bytes allocated: 247963744 
    Deviation   T5321FD(normal) bytes allocated:     -48.1 %
*** unexpected stat test failure for T5321FD(normal)

Unexpected results from:
TEST="T5321FD T5030 break021 T3064 parsing001 T5321Fun T3294"

SUMMARY for test run started at Thu Sep  8 22:48:46 2016 PDT
 0:00:09 spent to go through
       7 total tests, which gave rise to
      17 test cases, of which
      10 were skipped

       0 had missing libraries
       0 expected passes
       0 expected failures

       0 caused framework failures
       0 unexpected passes
       1 unexpected failures
       6 unexpected stat failures

Unexpected failures:
   ghci.debugger/scripts/break021.run  break021 [bad stdout] (ghci)

Unexpected stat failures:
   perf/compiler/T3294.run       T3294 [stat too good] (normal)
   perf/compiler/T3064.run       T3064 [stat too good] (normal)
   perf/compiler/T5030.run       T5030 [stat not good enough] (normal)
   perf/compiler/parsing001.run  parsing001 [stat not good enough] (normal)
   perf/compiler/T5321Fun.run    T5321Fun [stat too good] (normal)
   perf/compiler/T5321FD.run     T5321FD [stat too good] (normal)

I assume that the stat ones are just the result of moving things around? Unfortunately I'm kind of lost on what to do with these failures.. I'll try running validate on my Linux machine as well to see if there are any discrepancies.

Ahh, right, sorry @richardfung; I had meant to notify you of that ticket.

The performance changes are quite expected and we just need to update the expected allocations values. These are given in the all.T files associated with the respective tests. In addition to updating the expected number itself, it is convention to add a comment giving the date of the change and its cause.

richardfung updated this revision to Diff 8699.Sep 10 2016, 3:35 PM
richardfung edited edge metadata.

Updated the perf tests

I updated the perf tests to match the current memory usage! However, there's still one test failing and I'm not entirely sure why this is happening:

make test TEST="break021"

gives..

=====> break021(ghci) 1 of 1 [0, 0, 0]
cd "./ghci.debugger/scripts/break021.run" && HC="/Users/richard/ghc/inplace/test   spaces/ghc-stage2" HC_OPTS="-dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups " "/Users/richard/ghc/inplace/test   spaces/ghc-stage2" -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups  --interactive -v0 -ignore-dot-ghci -fno-ghci-history +RTS -I0.1 -RTS  -ignore-dot-ghci< break021.script
Actual stdout output differs from expected:
--- ./ghci.debugger/scripts/break021.run/break021.stdout.normalised	2016-09-10 13:37:07.000000000 -0700
+++ ./ghci.debugger/scripts/break021.run/break021.run.stdout.normalised	2016-09-10 13:37:07.000000000 -0700
@@ -17,7 +17,7 @@
       ^^^^^^^
 11    line2 0
 Stopped in Main.line1, break020.hs:3:11-19
-_result :: IO () = _
+_result :: m () = _
 2  
 3  line1 _ = return ()
              ^^^^^^^^^
@@ -29,7 +29,7 @@
       ^^^^^^^
 12    in_another_decl 0
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
@@ -55,7 +55,7 @@
                           ^^^^^^^
 7                         line2 0
 Stopped in Main.line1, break020.hs:3:11-19
-_result :: IO () = _
+_result :: m () = _
 2  
 3  line1 _ = return ()
              ^^^^^^^^^
@@ -67,7 +67,7 @@
                           ^^^^^^^
 8  
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
@@ -85,7 +85,7 @@
       ^^^^^^^
 15    return ()
 Stopped in Main.line2, break020.hs:4:11-19
-_result :: IO () = _
+_result :: m () = _
 3  line1 _ = return ()
 4  line2 _ = return ()
              ^^^^^^^^^
*** unexpected failure for break021(ghci)

Unexpected results from:
TEST="break021"

I'd be happy to just change the stdout but.. is this really okay? It seems like some debugging information might be getting lost.

Hmmm, very interesting. I have also noticed similar sort of changes in some patches I have in flight and haven't gotten to the bottom of it yet. It's hard to imagine how the result could really be more general now. @simonmar, do you have any intuition for what is going on here?

simonmar requested changes to this revision.Sep 14 2016, 10:43 AM
simonmar added a reviewer: simonmar.

Thanks for subscribing me to this diff :) The changes to the breakpoint tests are probably benign, but I have other concerns. I care a lot about unoptimized compile-time performance - couldn't this make things worse by forcing GHC to read all the interface pragmas from interface files, even with -O0? Also, this diff is disabling all inlining with -O0, which will mean we generate possibly much worse code because there will be no inlining within a module.

testsuite/tests/perf/compiler/all.T
402

eeek! what happened here?

This revision now requires changes to proceed.Sep 14 2016, 10:43 AM

Thanks for subscribing me to this diff :) The changes to the breakpoint tests are probably benign, but I have other concerns. I care a lot about unoptimized compile-time performance - couldn't this make things worse by forcing GHC to read all the interface pragmas from interface files, even with -O0? Also, this diff is disabling all inlining with -O0, which will mean we generate possibly much worse code because there will be no inlining within a module.

Thanks for taking a look at it!

(Note this is my first ticket so I probably don't know what I'm talking about)

I think that you are right about the performance issue with respect to reading the interface pragmas even with -O0. This seems to be okay to me because if people are really concerned then they can still fall back to the old performance and behavior by using -fignore-interface-pragmas right?

I suppose the other option to fix the issue with --make is to decide whether to read the interface pragmas based off the highest optimization level of the importing modules? I don't know how difficult that would be but that sounds complicated to me.

Unfortunately I don't know what we can do about inlining within a module. Hopefully someone else can chime in.

I'd rather not read the interface pragmas with -O0, because it means that even in GHCi we'll be reading those (possibly large) unfoldings and converting them into their internal representations even though they're never used.

Reasonable solutions:

  1. Lazily load the pragma info, so that it doesn't cost anything if we don't use it. The simplifier should use -fignore-interface-pragmas to decide whether to use the pragma info from external Ids or not.
  2. Predict whether we'll need the pragma info by determining whether *any* module in the current set will be compiled with optimisation turned on. This info is available after we've done the downsweep in the compilation manager. This approach doesn't really work in general because a user of the GHC API might load more modules later with -O and we can't predict that, but it fixes the common case.

(2) is easier but (1) is better. Take your pick :)

Perhaps we should land this correctness change first, then @richardfung can (hopefully) continue on the performance task that you pointed out?

Hmm, I'm not all that keen on introducing a regression to fix this bug, unless we commit to fixing the regression before a release. But then, why not hold off until we have a proper fix?

richardfung abandoned this revision.Sep 26 2016, 4:15 PM

I will try Simon's suggested fix in a new diff. Thanks for all the feedback!