Opened 14 years ago

Closed 8 years ago

Last modified 6 years ago

#437 closed bug (fixed)

Recompilation check should include flags

Reported by: simonpj Owned by: dterei
Priority: low Milestone: 7.4.1
Component: Compiler Version: 6.4.1
Keywords: Cc: Bulat.Ziganshin@…, ganesh.sittampalam@…, davidterei@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case: mod175
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by simonmar)

GHC skips recompilation of a module if the module code 
hasn't changed, even if the flags in use *have* changed.  
A benign version is that switching on -O won't recompile 
modules that were compiled without -O.  But here's a 
nastier version: changing the -main-is flag can lead to an 
obscure link error.  Details below supplied by Niels 
[cpjvelde@cs.uu.nl]

Simon

Actually, i reproduced it now and the reason is a bit 
different. I have an
application Test and Test2. They both have a main 
function. Test imports Test2
for some other function. So this is how those files look 
like:

~/pancake > cat Test.hs
module Test where
import Test2 hiding (main)

main = doit

~/pancake > cat Test2.hs
module Test2 where

doit = print "Test2.doit"
main = print "Test2.main"

I then first compile the first app:
~/pancake > ghc --make -main-is Test.main Test.hs
Chasing modules from: Test.hs
Compiling Test2            ( ./Test2.hs, ./Test2.o )
Compiling Test             ( Test.hs, Test.o )
Linking ...

then i compile the second app:
~/pancake > ghc --make -main-is Test2.main Test2.hs
Chasing modules from: Test2.hs
Skipping  Test2            ( Test2.hs, Test2.o )
Linking ...
/usr/lib/ghc-6.4/libHSrts.a(Main.o)(.text+0xe): In function 
`main':
: undefined reference to `__stginit_ZCMain'
/usr/lib/ghc-6.4/libHSrts.a(Main.o)(.text+0x28): In 
function `main':
: undefined reference to `ZCMain_main_closure'
collect2: ld returned 1 exit status

So I guess generating Test2.o the first time and using -
main-is renamed the main
in Test2.o . Since it is not recompiled when I want to 
compile the second app,
it fails because it cant find the main...I thought providing 
a -main-is flag
again when compiling the second app would somehow 
force recompilation of Test2.o
or at least fixing the 'renaming' that the first compilation 
of Test2.o had 
caused.

greetings, Niels.


Change History (28)

comment:1 Changed 14 years ago by simonmar

Component: NoneCompiler
Description: modified (diff)
Version: None6.4.1

comment:2 Changed 13 years ago by igloo

Architecture: Unknown
difficulty: Unknown
Milestone: 6.8
Operating System: Unknown
Test Case: mod175

comment:3 Changed 13 years ago by igloo

Owner: nobody deleted
Status: assignednew

See also #106.

comment:4 Changed 13 years ago by guest

Cc: Bulat.Ziganshin@… added

comment:5 Changed 13 years ago by guest

Cc: ganesh@… added

comment:6 Changed 13 years ago by guest

Cc: ganesh.sittampalam@… added; ganesh@… removed

comment:7 Changed 12 years ago by igloo

Milestone: 6.8 branch6.10 branch

comment:8 Changed 11 years ago by simonmar

Architecture: UnknownUnknown/Multiple

comment:9 Changed 11 years ago by simonmar

Operating System: UnknownUnknown/Multiple

comment:10 Changed 11 years ago by igloo

Milestone: 6.10 branch6.12 branch

comment:11 Changed 10 years ago by simonmar

Priority: lownormal
Type of failure: Other

comment:12 Changed 10 years ago by igloo

Milestone: 6.12 branch6.12.3

comment:13 Changed 10 years ago by igloo

Milestone: 6.12.36.14.1
Priority: normallow

comment:14 Changed 9 years ago by igloo

Milestone: 7.0.17.0.2

comment:15 Changed 9 years ago by igloo

Milestone: 7.0.27.2.1

comment:16 Changed 9 years ago by dterei

Was doing some work in recompilation avoidance for the SafeHaskell work and came over this bug. I was thinking, the easiest way to fix this would be to store a new fingerprint in the interface file for a module that is a hash of the flags that if changed should trigger recompilation. So '-main-is', probably '-O' should be part of the hash but '-Wall' shouldn't for example. If this sounds like a reasonable plan then I would be happy to code it up since it would be useful somewhat to the SafeHaskell work. Recompilation of a module on this condition I don't think should trigger recompilation of modules depending on it though.

comment:17 Changed 9 years ago by dterei

Cc: davidterei@… added

comment:18 in reply to:  16 Changed 9 years ago by simonmar

Replying to dterei:

I was thinking, the easiest way to fix this would be to store a new fingerprint in the interface file for a module that is a hash of the flags that if changed should trigger recompilation. So '-main-is', probably '-O' should be part of the hash but '-Wall' shouldn't for example. If this sounds like a reasonable plan then I would be happy to code it up since it would be useful somewhat to the SafeHaskell work. Recompilation of a module on this condition I don't think should trigger recompilation of modules depending on it though.

Sounds good. You would need to be careful to normalise the flags (sort them), and as you say filter out any flags that shouldn't trigger recompilation. It's not clear to me that changing optimisation settings should trigger recompilation: for example, you might have a large program compiled unoptimised, and then decide that you want to optimise just one or two modules, so you remove a few .o files, add -O and recompile.

Rather than using the text of the flags themselves, you might want to serialise the internal representation, as we do for the linker flags (see DriverPipeline.getLinkInfo).

comment:19 Changed 9 years ago by dterei

Owner: set to dterei

comment:20 Changed 9 years ago by igloo

Flag ordering is important (e.g. -O0 -O is not the same as -O -O0). Also, some flags are equivalent to others.

Rather than storing the set of flags (-O etc), it might be better to use the set of enabled DynFlags (Opt_Specialise etc) and ExtensionFlags.

It might also make sense to split DynFlag into 2 (or more) types, so e.g. Opt_D_dump* and Opt_Warn* are in a different type than the flags that actually affect optimisation etc. Then the recompilation checker can just look at the set of values of the second type (and the ExtensionFlag type). That may mean it sometimes recompiles when it isn't strictly necessary, but I don't think that's a major problem.

comment:21 in reply to:  20 Changed 9 years ago by simonmar

Replying to igloo:

Flag ordering is important (e.g. -O0 -O is not the same as -O -O0). Also, some flags are equivalent to others.

You're right, literally sorting the flags would be wrong. I had in mind serialising DynFlags, but making sure that the DynFlag list is sorted to normalise it, and omitting things that don't affect the compilation result, plus throwing in a few things that aren't in DynFlags (static flags).

Rather than storing the set of flags (-O etc), it might be better to use the set of enabled DynFlags (Opt_Specialise etc) and ExtensionFlags.

Right.

comment:22 Changed 8 years ago by igloo

Milestone: 7.2.17.4.1

comment:23 Changed 8 years ago by dterei

So I'm finally actually working on this. There are some questions though:

  • Should optmisation flag changes be candidates for recompilation?

  • There is the whole hierarchy of hashes GHC uses (Interface File Hash, ABI Hash and then individual more specialised hashes). I've created a new flags hash. Should the ABI and Interface File Hash be dependent on the flags hash? The interface file hash should be. However I think for the ABI hash we could do something a little more complex if its worth the effort. There are probably some flags that if changed we want it to trigger a recompilation of the module M but we don't want the modules ABI hash to change and trigger recompilation of modules depending on M. However some flags like '--main-is' should change M's ABI hash. Is it worth differentiating between these two flag cases?

comment:24 Changed 8 years ago by dterei

Other flags not sure if we should be including in the hash:

  • includePaths :: [String]
  • libraryPaths :: [String]
  • frameworkPaths :: [String]
  • cmdlineFrameworks :: [String]
  • rtsOpts :: Maybe String
  • rtsOptsEnabled :: RtsOptsEnabled
  • hpcDir :: String
  • settings :: Settings -- contains the programs and options to use for various phases
  • packageFlags :: [PackageFlag]
  • language :: [Maybe Language]

comment:25 Changed 8 years ago by davidterei@…

commit 55991bf6631c95b620aec23ae50d25968f4a7f6e

Author: David Terei <davidterei@gmail.com>
Date:   Wed Nov 9 22:53:07 2011 -0800

    Fix #437: recompilation check includes flags

 compiler/ghc.cabal.in         |    1 +
 compiler/iface/BinIface.hs    |    4 +
 compiler/iface/FlagChecker.hs |  143 ++++++++++++++++++++++++++++++++
 compiler/iface/LoadIface.lhs  |    1 +
 compiler/iface/MkIface.lhs    |  184 ++++++++++++++++++++++++-----------------
 compiler/main/HscTypes.lhs    |    5 +-
 compiler/utils/Binary.hs      |   32 +++++---
 7 files changed, 282 insertions(+), 88 deletions(-)

comment:26 Changed 8 years ago by dterei

Resolution: Nonefixed
Status: newclosed

Fixed. But there is a question of which flags should or shouldn't be included in the check.

comment:27 Changed 8 years ago by marlowsd@…

commit 9df7f9b4022ffda25f004cb099de358d7d240fc1

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Thu Nov 10 11:14:20 2011 +0000

    Add more flags to the recompilation check (#437)
    
    Now included:
        - all language flags
        - all filename-related flags (-i, -osuf, -hidir etc.)
        - all CPP-related flags (-I, -D, -U)

 compiler/iface/FlagChecker.hs |   40 +++++++++++++++++++++-------------------
 compiler/iface/MkIface.lhs    |    3 +--
 compiler/main/DynFlags.hs     |    3 ++-
 3 files changed, 24 insertions(+), 22 deletions(-)

comment:28 Changed 6 years ago by Ian Lynagh <igloo@…>

In 49ca979dc07fb0bfb9a790630cb2eb41750a96bd/ghc:

mod174/mod175 are broken: trac bugs #414 and #437
Note: See TracTickets for help on using tickets.