Opened 4 years ago

Closed 3 years ago

#3843 closed task (fixed)

Merge plugins into HEAD

Reported by: dreixel Owned by:
Priority: high Milestone: 7.2.1
Component: Compiler Version: 6.13
Keywords: Cc: jpm@…, pumpkingod@…, batterseapower@…, michael.dever2@…, mad.one@…, dmp@…, shelarcy@…, tanaka.hideyuki@…, leather@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty:
Test Case: Blocked By:
Blocking: Related Tickets:

Description (last modified by simonpj)

This ticket serves to track the progress of the integration of core plugins in GHC (see http://www.haskell.org/pipermail/glasgow-haskell-users/2010-January/018294.html). If you are interested in having Max Bolingbroke's work merged into the mainline, please add yourself to the CC list of this ticket, preferably also commenting saying why you want plugins.

This wiki page describes the design: NewPlugins

Attachments (4)

ghc_plugins_support_2010_11_19.dpatch (33.9 KB) - added by thoughtpolice 3 years ago.
Patch implementing support for compiler plugins; dated 2010-11-19
ghc_plugins_support_2010_11_19.2.dpatch (34.0 KB) - added by thoughtpolice 3 years ago.
Patch implementing support for compiler plugins; dated 2010-11-19
3843.zip (6.4 KB) - added by batterseapower 3 years ago.
3843-v2.zip (5.7 KB) - added by batterseapower 3 years ago.

Download all attachments as: .zip

Change History (64)

comment:1 Changed 4 years ago by dreixel

I would want core plugins to experiment with certain optimizations for generic programs, mostly related to inlining. Since this is rather experimental, plugins would allow easy modification of the core-to-core pass for testing (rather than having to recompile the compiler with an extra hard-coded core-to-core pass).

comment:2 Changed 4 years ago by igloo

  • Type changed from merge to task

comment:3 Changed 4 years ago by pumpkin

  • Cc pumpkingod@… added

I'd like to use plugins to help show the programmer what rewrite rules are firing and where. The current most verbose simplifier output doesn't provide enough information to allow an external program to figure out what was rewritten and where (it just says that a rule fired and gives the new version of the code). This would help to find performance bugs in fusion-heavy code for example.

comment:4 Changed 4 years ago by simonpj

A core-to-core plugin pass does just what it sounds like: transforms the program to a new program. I don't know a good way for a plugin to monitor what another core-to-core pass is doing; in this case the rewrite rules, which are fired by the Simplifier. It's not clear to me how to do that. What would it mean to "figure out what was rewritten and where", when the whole program is being radically transformed?

It's a good goal, but I don't currently know how to achieve it.

There is a (slightly moribund) core-to-core pass, enabled by -frule-check, that looks for places where a rule nearly (but not quite) applies, and reports those. So it might be a start for what you want. The code is in Rules.ruleCheckProgram.

Simon

comment:5 Changed 4 years ago by batterseapower

Patch attached. NB: the patch is against a fairly old GHC at this point!

My original comments with the patch are below this line:

What's included:

  • Dynamic loading of Core passes
  • -plg command line flag to load plugins
  • -P PluginName:option command line flag to supply options to them
  • Support for using plugins which are not installed in the package DB

(i.e. I have write a Plugin.hs and use it directly to compile Foo.hs
in the same directory)

  • Tests for plugin system

The annotation system and CoreMonad? were already merged, so this patch is quite manageable.

I've run the full testsuite against the tree, and there are no additional failures compared to HEAD.

Outstanding issues:

  • Need updated documentation to reflect the new plugins story (no phase system). I'll write this if you approve of the new design for how plugins get installed.
  • It probably only works on OS X because I'm using Apple linker options to export package GHC from the GHC executable. We probably need shared library support before it works on Windows and Linux. However, I've guessed at some linker options that are suitable for those platforms (see ghc-bin), so there is a small possibility it will

work.

My two simple example plugins (strict-plugin, cse-plugin - available on code.haskell.org) both work correctly. My complex example plugin segfaults during compilation, but this has always been true. I am not sure how to debug this, and it might be related to my linker hacks. For this reason it might be best to merge the plugins patch so I can stop maintaining the branch but keep it on the down-low until we find some time to sort this out AFTER shared libaries are in.

comment:6 Changed 4 years ago by batterseapower

  • Cc batterseapower@… added

Actually, there is a file size limit of 256kB so I can't attach it.

I've restored the patches to a URL at the Cambridge computer lab though: http://www.cl.cam.ac.uk/~mb566/files/Plugins-Patches.zip

(The testsuite patch is 75MB for some reason!)

comment:7 Changed 4 years ago by simonpj

Here's a link describing how Scala suppports plugins: http://www.scala-lang.org/node/140

comment:8 Changed 4 years ago by igloo

  • Milestone set to 6.14.1

comment:9 Changed 4 years ago by michaelcdever

  • Cc michael.dever2@… added

comment:10 Changed 3 years ago by thoughtpolice

  • Cc mad.one@… added

I have taken the liberty of updating Max's patch to work with the latest GHC HEAD (7.1.x at time of this writing) after talking with him.

It is implemented in the following patch:

Fri Nov 19 18:44:58 CST 2010  austin seipp <as@hacks.yi.org>
  * Implement support for writing and loading plugins for GHC
  Based on Max Bolingbroke's original patch for implementing plugin support in GHC,
  which can be gotten from here: http://hackage.haskell.org/trac/ghc/ticket/3843
  
  A little bit of the linker interface changed and the simplifier interface changed
  inbetween now and then. This patch brings most of the work Max did up to date although
  there may be a few loose ends. The interface for plugins that rewrite core hasn't changed
  (that is, most people will be writing passes of type [CoreBind] -> CoreM [CoreBind])
  
  Plugins are registered the same way as they were previously: plugin modules provide an installation
  function of type [CoreToDo] -> CoreM [CoreToDo]
  
  Changes:
   * -plg and -P flags replaced by -fplugin and -fplugin-arg (consistent with what GCC uses)
  

    M ./compiler/basicTypes/Module.lhs -1 +3
    M ./compiler/ghc.cabal.in +4
    M ./compiler/ghci/Linker.lhs -2 +39
    M ./compiler/main/DynFlags.hs -1 +24
    A ./compiler/main/DynamicLoading.hs
    A ./compiler/main/GHCPlugins.hs
    A ./compiler/main/LoadPlugins.lhs
    A ./compiler/main/Plugins.lhs
    A ./compiler/main/Plugins.lhs-boot
    M ./compiler/prelude/PrelNames.lhs -1 +20
    M ./compiler/simplCore/CoreMonad.lhs -1 +26
    M ./compiler/simplCore/SimplCore.lhs -3 +39

I currently haven't ported over the patches Max made to the testsuite, although with the above patch you can use my slightly modified version of Max's CSE plugin, which can be found here:

https://github.com/thoughtpolice/cse-ghc-plugin

It seems to work as expected.

Patch will be attached. I will continue to update this with patches for fixes and eventually the testsuite programs. I am willing to help maintain this functionality and make improvements fixes, essentially because I want to see it finally.merged into GHC mainline. It is early in the development cycle for GHC 7.2, so it would be wonderful if we could get it merged early so there is time to make changes and fix bugs for a while.

Later on perhaps, I would like to extend the Plugins functionality to encompass writing Cmm plugins and perhaps inevitably new backends (after the new integrated code generator lands,) although that will require design on my part and input from many people (I would however, like to get this merged soon!)

Comments welcome.

Changed 3 years ago by thoughtpolice

Patch implementing support for compiler plugins; dated 2010-11-19

Changed 3 years ago by thoughtpolice

Patch implementing support for compiler plugins; dated 2010-11-19

comment:11 Changed 3 years ago by simonpj

The second patch ghc_plugins_support_2010_11_19.2.dpatch is just a comment update and a warning fix. Implemented by Austin Seipp.

comment:12 Changed 3 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:13 Changed 3 years ago by dmp

  • Cc dmp@… added

comment:14 Changed 3 years ago by simonpj

  • Description modified (diff)

comment:15 Changed 3 years ago by shelarcy

  • Cc shelarcy@… added

I'd like to use some plugins when plugins is merged to ghc HEAD branch. So, I write an opinion about plugin users' point of view.

I want to use strict-ghc-plugins. Off course I know that ghc's profiler supports to find space leak problem. But I want adding strict-ghc-plugins in my toolbox to solve space leak problem.

https://github.com/thoughtpolice/strict-ghc-plugin

I want to use supercompiler plugin. I think Neil Mitchell will provide plugin version of supero after plugins is merged. (From this mail, Neil is interested in plugin. http://www.haskell.org/pipermail/glasgow-haskell-users/2011-January/019901.html So, I think he will provide plugin version of supero.) I also think that somebody else can provide alternative supercompilation system as a plugin. I'd like to trying to use these system for optimizing my program.

http://hackage.haskell.org/package/supero

And if someone write plugin version of Static contract checking, then I want to use that. I think merging Static contract checking is difficult. Because compiler becomes slow when merging Static contract checking. But if plugins would be merged, we could make plugin version more easily.

http://research.microsoft.com/en-us/um/people/simonpj/papers/verify/index.htm

comment:16 Changed 3 years ago by shelarcy

One problem is that there is no patch for Cabal. I think we need Cabal support for plugins. cse-ghc-plugins and strict-ghc-plugins require to link GHC API. This means that code bloat occurs when building package that depends on plugin. If we want to avoid code bloat, we must split plugin into annotation package and plugin package. This is not good.

Does anyone has an idea that solves this problem?

This is just Cabal's problem. So, I don't know we should discuss here, or we should discuss on Hackage project's Ticket after plugin is merged in GHC HEAD.

comment:17 Changed 3 years ago by tanakh

  • Cc tanaka.hideyuki@… added

comment:18 Changed 3 years ago by thoughtpolice

Hi shelarcy (sorry for the delay in reply!,)

I think Supercompilation seems possible from a plugin perspective. From what I can tell, Simon has implemented a flag called -fexpose-all-unfoldings in GHC which will put all unfoldings for everything in interface files - this is great for whole program work of any sorts or supercompilation, like the work Neil or Max & Simon are doing.

I haven't looked at the implementation of expose-all-unfoldings myself however. Simon, can you comment on the possibility of utilizing expose-all-unfoldings from CoreM? It seems as though ModGuts exposes most of the necessary dependency information for the currently compiled module, to find what's needed for something like this, meaning we *could* use a ModGutsToBindsPluginPass (or, if we're daring, a ModGutsToModGutsPluginPass.) I don't know if this is enough, however (I do not have enough GHC-fu.)

As for the static contract checking work, I honestly have no idea. I have not read Dana's paper, but from the abstract it seems it works by symbolic execution with the contracts in mind - I don't think this happens at the Core level however from a quick glance, and currently that is all the plugin work encompasses. It does not expose all the internal phases of GHC - only a limited API allowing us to analyze/rewrite core bindings at the moment. I think this is outside of the scope of the actual plugins work, but I could be wrong (again, Simon should say something here because I could be completely wrong.)

Finally, as for the Cabal concern, I think you may be right, but my intuition tells me that as a client, you would only depend on the actual datatype definition for the annotation, and not any code which *directly* touches the GHC package itself. I would expect that the linker would remove the GHC package as dead code during the final link step. I have not tested this myself, but I should in any case. Thanks for pointing this out, I'll get back to you when I get the chance.

comment:19 Changed 3 years ago by batterseapower

ESC and supercompilation could be implemented by plugin passes. I think I even added an API to report warnings from Core plugins with ESC in mind. Not sure if that made it into the patch above.

If you want linking to work well you need to break annotations out into a separate package. As it stands, because of how GHC handles module initialisation, the linker is unable to remove TONS of dead Haskell code. So for example if you use QuasiQuoting? then your executables bloat because we link in most of GHC even though you only needed it it at compiled time! There is an open ticket about this (search for "Quasiquoting without bloating")

comment:20 Changed 3 years ago by spl

  • Cc leather@… added

comment:21 Changed 3 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:22 follow-up: Changed 3 years ago by thoughtpolice

I'm moving these patches to git as we speak and making sure everything still works. The initial commit containing the verboten work from darcs will (soon) be available here:

https://github.com/thoughtpolice/ghc/tree/plugins

The patches don't affect anything other than the main compiler repository. I've been busy and still need testsuite patches. :/

I tested this a while ago before the VCS switchover (the SF bay hackathon,) and there seemed to be failures on OS X. I'm investigating them. I'd also like to test on windows before they get merged.

@shelarcy, recently, Simon Marlow refactored the way module initialization is done, see this commit: a52ff7619e8b7d74a9d933d922eeea49f580bca8 - the result is that plugins should no longer bloat the executables that use, say, module annotations. Thus you should be able to safely include both annotations and GHC plugins themselves in the same package.

There's some work to be done but I'd still like to get this done in time for 7.2.1 hopefully.

comment:23 in reply to: ↑ 22 Changed 3 years ago by shelarcy

Replying to thoughtpolice:

the result is that plugins should no longer bloat the executables that use, say, module annotations. Thus you should be able to safely include both annotations and GHC plugins themselves in the same package.

Great! Thank you for your information.

comment:24 Changed 3 years ago by thoughtpolice

I've tested the patches on my OS X machine (64bit GHC, HEAD built with 7.0.3) and the linker issue I previously encountered seemed to go away (probably someone fixed a bug.)

All of the plugins that were initially developed still work, and the following 3 plugins all work and have tests when built with GHC + the plugin patches (I just fixed Max's unrolling plugin):

https://github.com/thoughtpolice/unroll-plugin

https://github.com/thoughtpolice/cse-ghc-plugin

https://github.com/thoughtpolice/strict-ghc-plugin

So, I'll get working on adding some basic testsuite patches soon.

comment:25 follow-up: Changed 3 years ago by simonpj

OK I have pushed a patch to HEAD. My starting point was Austin's patch, in turn based on Max's work, but needless to say I did some refactoring. Mainly, I simplified it. For example, instead of three constructors for the plugin pass (corresponding to different interfaces), there is one, of type ModGuts -> CoreM ModGuts, plus some impedence matching functions. I also used fewer different modules, and removed module loops. I made a quck test of the result with the CSE pass above (changed a bit to match the new API).

So, Austin, could I ask you to do the final bits?

  • Add some words to the user manual to describe the new capability. This can be relatively brief reference material; point to the wiki for examples and further elaboration.
  • Update the wiki page to accurately reflect the final version. Presumably NewPlugins
  • Add at least one test to the testsuite. (Should test the flag-passing mechamism.)

Many thanks.

commit 592def09c4f87f517b31aa4c4cec51fc8764a859
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Thu Jun 16 17:22:47 2011 +0100

    Add dynamically-linked plugins (see Trac #3843)
    
    This patch was originally developed by Max Bolingbroke, and worked on
    further by Austin Seipp.  It allows you to write a Core-to-Core pass
    and have it dynamically linked into an otherwise-unmodified GHC, and
    run at a place you specify in the Core optimisation pipeline.
    
    Main components:
      - CoreMonad: new types Plugin, PluginPass
                   plus a new constructor CoreDoPluginPass in CoreToDo
    
      - SimplCore: stuff to dynamically load any plugins, splice
        them into the core-to-core pipeline, and invoke them
    
      - Move "getCoreToDo :: DynFlags -> [CoreToDo]"
          which constructs the main core-to-core pipeline
          from CoreMonad to SimplCore
        SimplCore is the driver for the optimisation pipeline, and it
        makes more sense to have the pipeline construction in the driver
        not in the infrastructure module.
    
      - New module DynamicLoading: invoked by SimplCore to load any plugins
        Some consequential changes in Linker.
    
      - New module GhcPlugins: this should be imported by plugin modules; it
        it not used by GHC itself.

 compiler/basicTypes/Module.lhs   |    4 +-
 compiler/ghc.cabal.in            |    2 +
 compiler/ghci/Linker.lhs         |   41 +++++-
 compiler/main/DynFlags.hs        |   23 +++
 compiler/main/DynamicLoading.hs  |  150 +++++++++++++++++++
 compiler/main/GhcPlugins.hs      |   83 +++++++++++
 compiler/prelude/PrelNames.lhs   |   18 +++
 compiler/simplCore/CoreMonad.lhs |  261 +++++++++-------------------------
 compiler/simplCore/SimplCore.lhs |  294 ++++++++++++++++++++++++++++++++++++--
 9 files changed, 665 insertions(+), 211 deletions(-)

comment:26 Changed 3 years ago by thoughtpolice

Simon,

Sure. Thanks for taking the time to integrate this! I'm working on another ticket anyway, so I'll set that aside for today and write up a manual entry, put a few tests in, and update the wiki page. I'll also update the CSE/Strictness/Unrolling packages for the new API. Two quick questions:

  • Should I also add a blurb on a release notes to state that plugins are new in 7.2.1? Or will Igloo take care of this at release-time?
  • What section should I put the GHC plugins information under, if there's any preference? The best candidate seems to be section 4, "Using GHC." I suppose just putting it in its own subsection here near the end would probably be the best.

Glad to see this ticket closed. Thanks simon!

comment:27 Changed 3 years ago by simonpj

For the release notes, why don't you write a couple of suggested sentences and send them to Ian? I don't konw if there's a draft 7.2 release-notes files yet, even.

For the manual, I've just taken a look. I think the best thing would really be this:

  • Start a whole new chapter, perhaps after the FFI chapter, entitled "Extending GHC". The idea is that this chapter would cover
    • Using GHC as a library; the GHC API and package ghc
    • Plugins (which of course use package ghc)
    • Annotations (currently in 7.3.6)
  • Not forgetting to update the flag descriptions in Section 4

Does that make sense? Doubtless

comment:28 follow-up: Changed 3 years ago by batterseapower

I had to change the flags to "-fplugin Mod" and "-fplugin-arg Mod:foo-bar" - i.e. with a manifest space between the -f bit and the actual argument. This is because -fplugin-arg=Mod:foo-bar was being parsed as a plugin whose module name was literally -arg=Mod:foo-bar.

Furthermore, my supercompilation plugin dies with a panic about static flags on OS X, which makes me suspect that there are still linking issues to be resolved before plugins will work properly.. I'm about to experiment with this, stay tuned.

comment:29 in reply to: ↑ 28 ; follow-up: Changed 3 years ago by thoughtpolice

Sorry for the delay. I have some manual changes sitting on my machine at home. When I get the chance later I'll make sure there aren't any atrocious typos or weirdness in the HTML output, and put a branch on github. I'll follow up with a few tests.

Replying to batterseapower:

I had to change the flags to "-fplugin Mod" and "-fplugin-arg Mod:foo-bar" - i.e. with a manifest space between the -f bit and the actual argument. This is because -fplugin-arg=Mod:foo-bar was being parsed as a plugin whose module name was literally -arg=Mod:foo-bar.

Furthermore, my supercompilation plugin dies with a panic about static flags on OS X, which makes me suspect that there are still linking issues to be resolved before plugins will work properly.. I'm about to experiment with this, stay tuned.

I can look into this later tonight when I get home if you'd like (if it's easily reproducible.) Is this a 32bit GHC or 64bit, out of curiosity? (I use the 64bit build typically.) I ran into linker issues a few months ago that ended up mysteriously going away after updating my tree at one point, so it's possible other interim changes have borked things a little.

comment:30 in reply to: ↑ 29 Changed 3 years ago by batterseapower

Not all of my patch seems to have landed. Support for home-package plugins didn't make it in, along with my changes to the linker flags. Is there a reason for the removal of home-package plugin support?

thoughtpolice: I've ported the tests from my old patch forward. I'll commit when the last one stops failing (it depended on home-package plugins).

comment:31 Changed 3 years ago by simonpj

I'm not sure what "support for home-package plugins" is. I certainly simplified the patch when testing it; I removed several small modules, eliminated unnecessary module loops; simplified data types. But I didn't know that I had removed any functionality. If so, apologies. Happy to discuss.

comment:32 Changed 3 years ago by batterseapower

simon: I've restored the home-package plugin support. This is basically just the ability to write:

{-# OPTIONS_GHC -fplugin Foo #-}
module Bar
...

And then if you use ghc --make on Bar.hs Foo.hs, will be compiled if is not already. Have a look at e49dae36a00b2af8f6ad583dd24f9bacf5711242 if you want to see what is required.

~

thoughtpolice: As I thought, linking is not working properly: see the newly-commited test plugins06. In this test, the plugin seqs the static flags, which provokes the error:

ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.1.20110630 for i386-apple-darwin):
	Static flags have not been initialised!
        Please call GHC.newSession or GHC.parseStaticFlags early enough.

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

This is because a new instance of the GHC package has been loaded, which does not have its staticFlags initialised since it does not share state with the initial copy.

The fix is to do export-dynamic and change Linker.hs to get ghc package symbols from the current executable (which is what I did in my original patch), or use dynamic linking for the ghc package in which case everything will work.

comment:33 Changed 3 years ago by simonpj

Great. If you two work it out, let me know what to commit.

comment:34 Changed 3 years ago by simonpj

Guys, I'm getting validate failures from plugin tests. What's up?

Unexpected failures:
   plugins             plugins01 [bad exit code] (normal)
   plugins             plugins02 [stderr mismatch] (normal)
   plugins             plugins03 [stderr mismatch] (normal)

Here's some log stuff

=====> plugins01(normal) 2731 of 2829 [0, 3, 1]
cd ./plugins && $MAKE -s --no-print-directory plugins01    </dev/null >plugins01.run.stdout 2>plugins01.run.stderr
Wrong exit code (expected 0 , actual 2 )
Stdout:

Stderr:
make[4]: /64playpen/simonpj/builds/validate-HEAD/bindisttest/install: Command not found

---------------------
=====> plugins02(normal) 2732 of 2829 [0, 4, 1]
cd ./plugins && '/64playpen/simonpj/builds/validate-HEAD/bindisttest/install dir/bin/ghc' -fforce-recomp -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-conf -rtsopts  -c plugins02.hs  -package-conf simple-plugin/local.package.conf -fplugin Simple.BadlyTypedPlugin -package simple-plugin  >plugins02.comp.stderr 2>&1
Actual stderr output differs from expected:
--- ./plugins/plugins02.stderr.normalised	2011-07-01 07:55:21.530103012 +0100
+++ ./plugins/plugins02.comp.stderr.normalised	2011-07-01 07:55:21.530103012 +0100
@@ -1 +1 @@
-<command line>: The value Simple.BadlyTypedPlugin.plugin did not have the type CoreMonad.Plugin as required 
\ No newline at end of file
+ghc: can't find a package database at simple-plugin/local.package.conf 

comment:35 Changed 3 years ago by batterseapower

This is definitely my fault, though (obviously) it doesn't happen on my machine. I'm on it.

comment:36 Changed 3 years ago by igloo

It should happen if you use BINDIST=YES when running the tests. validate always does that.

You probably just need to quote $(HC_TEST) in the Makefile.

comment:37 Changed 3 years ago by batterseapower

Ian: yes that was the problem. I think I mistakenly validated another tree, not the one I actually pushed >_>

Fixing the linking issue on Windows is blocked by #5292. I've attached patches (in 3843.zip) that will work perfectly if #5292 is fixed (except that mingw32 will need to be changed from expect_broken), and already work perfectly (and are validated) on Linux and OS X.

Currently, applying this patch series breaks GHCis dynamic linker on Windows in a confusing way. The reason is the patch "Check EXE for any symbols after checking all loaded DLLs": the code added by this patch, combined with the GetProcAddress? behaviour described in #5292 causes GHCi's linker to resolve lots of functions that aren't actually available in the list of loaded DLLs (such as GetLastError?, recv, GetConsoleCP and many others) to the address to the *first export* from ghc.exe. This is obviously nonsense and leads to weirdness.

This weird behaviour only manifests if the export list overflows.

Changed 3 years ago by batterseapower

comment:38 Changed 3 years ago by daniel.is.fischer

One more $(TEST_HC) needs to be quoted: #5295

comment:39 Changed 3 years ago by daniel.is.fischer

Now plugins01 fails with

Actual stdout output differs from expected:
--- ./plugins/plugins01.stdout.normalised	2011-07-03 21:31:38.887000039 +0200
+++ ./plugins/plugins01.run.stdout.normalised	2011-07-03 21:31:38.887000039 +0200
@@ -1,7 +1,3 @@
-Writing new package config file... done.
-Reading package info from "dist/installed-pkg-config" ... done.
-Writing new package config file... done.
 Program Started
 Right
-Also Correct
 Program Ended
*** unexpected failure for plugins01(normal)

comment:40 Changed 3 years ago by batterseapower

Yep, that was in another one of my patches that I failed to push. I need to be more disciplined about validation - should probably have a tree I stage patches into, validate there and ONLY push from that tree.

Anyway, the plugins01 failure should be fixed now.

comment:41 Changed 3 years ago by thoughtpolice

Okay, current state of play - there are 2 commits for the documentation in this branch here:

https://github.com/thoughtpolice/ghc/tree/3843-plugin-docs

I talked to Max on IRC and he said he'd merge it into HEAD and the 7.2 branch. Currently plugins06 will still fail because of the linking issue (#5292). He is waiting for Simon M to give the go on this patch:

http://www.haskell.org/pipermail/cvs-ghc/2011-July/063587.html

so that plugins06 can also be fixed on windows. After this I think we should hopefully have everything in and ready to go.

comment:42 Changed 3 years ago by batterseapower

I've merged thoughpolice's doc changes and asked Ian to put them in 7.2.

Changed 3 years ago by batterseapower

comment:43 Changed 3 years ago by batterseapower

OK. The attached patches implement a fix for the plugins06 issue. Caveats:

  • This works on Windows and an earlier version (3483.zip) validated on Linux and OS X, but I have not specifically validated this version on those platforms. I'm pretty sure it should still work though.
  • A corresponding change needs to be made to the testsuite to mark plugins06 as passing on all platforms.
  • I don't think this patch works with a profiled/threaded GHC because I've hardcoded .a as the library suffix. Someone who knows more about the build system may know how to get _thr.a or _prof.a there instead?
  • If GHC changes to export just 200 more symbols a -O build of ghc-stage2 will start segfaulting in mysterious ways if you use GHCi due to strange Windows dynamic linker problems caused by DLLs with overflown export tables (due to my recent changes, ghc-stage2.exe only exports 65315 symbols now, but the maximum is only 65536!)
  • There is some more opportunity to cut down on the number of exports from ghc-stage2.exe. I don't currently prevent libHSghc-prim, libHSbinary and the RTS from being exported from ghc.exe, because they are only depended on transitively. How can I get hold of their version numbers in ghc.mk?
  • There is a code size impact from applying this patch. I can't say exactly but I think the growth is less than 20%
  • I'm actually seeing some validate failures, though I don't think they are the fault of this patch since they are all related to FFI support:
Unexpected failures:
   ffi/should_run  1679 [bad exit code] (normal)
   ffi/should_run  2469 [bad exit code] (normal)
   ffi/should_run  2594 [bad exit code] (normal)
   ffi/should_run  4038 [bad exit code] (normal)
   ffi/should_run  4221 [bad exit code] (normal)
   ffi/should_run  fed001 [bad exit code] (normal)
   ffi/should_run  ffi006 [bad exit code] (normal)
   ffi/should_run  ffi007 [bad exit code] (normal)
   ffi/should_run  ffi008 [bad exit code] (normal)
   ffi/should_run  ffi011 [bad exit code] (normal)
   ffi/should_run  ffi012 [bad exit code] (normal)
   ffi/should_run  ffi013 [bad exit code] (normal)
   ffi/should_run  ffi014 [bad exit code] (threaded1)
   ffi/should_run  ffi016 [bad exit code] (normal)
   ffi/should_run  ffi019 [bad exit code] (normal)
   rts             4850 [bad stdout] (normal)
   rts             5250 [bad exit code] (normal)
   rts             T4059 [bad exit code] (normal)

Can anyone reproduce this? Are these in fact my fault?

Please review the patch and advise on above points, particularly WRT to how to use the build system...

comment:44 Changed 3 years ago by daniel.is.fischer

I just tried 3843-v2 on linux x86_64, a full testsuite run gave

OVERALL SUMMARY for test run started at Do 7. Jul 20:43:33 CEST 2011
    2873 total tests, which gave rise to
    9783 test cases, of which
       0 caused framework failures
    1716 were skipped

    7827 expected passes
     227 expected failures
       6 unexpected passes
       7 unexpected failures

Unexpected passes:
   plugins  plugins06 (normal,hpc,optasm,ghci,threaded1,threaded2)

Unexpected failures:
   annotations/should_run  annrun01 [bad exit code] (ghci)
   codeGen/should_run      cgrun068 [exit code non-0] (dyn)
   dph/diophantine         dph-diophantine-opt [bad stdout] (normal,threaded1,threaded2)
   plugins                 plugins05 [exit code non-0] (dyn)
   typecheck/should_run    T4809 [exit code non-0] (dyn)

The unexpected failures are all expected (they fail anyway) except annrun01(ghci).
That one segfaults now, which it normally doesn't.
I'll build a new one and see whether that's caused by the patch.

comment:45 Changed 3 years ago by daniel.is.fischer

annrun01 passes without the 3843-v2, so it could well be the patch that broke it.

comment:46 Changed 3 years ago by batterseapower

daniel.is.fischer: Thanks for the investigation. I'll recheck the patches on Linux when I get a chance: that annrun01 failure certainly looks suspicious.

comment:47 Changed 3 years ago by igloo

  • Priority changed from normal to high
  • Status changed from new to patch

comment:48 in reply to: ↑ 25 Changed 3 years ago by simonpj

Replying to simonpj:

So, Austin, could I ask you to do the final bits?

  • Add some words to the user manual to describe the new capability. This can be relatively brief reference material; point to the wiki for examples and further elaboration.
  • Update the wiki page to accurately reflect the final version. Presumably NewPlugins
  • Add at least one test to the testsuite. (Should test the flag-passing mechamism.)

Austin: this was 5 weeks ago. Have you managed to make any progress on these points? It'd be embarrasing to release GHC 7.2 with a feature that isn't even mentioned in the user manual!

Also Max, Austin: can you update us with the status of the linking issues.

In the documentation do we need to say anything in particular about "home package plugins"?

Thanks, Simon

comment:49 Changed 3 years ago by thoughtpolice

The documentation has been in since about 2 weeks ago (commit 495e7dbec9828b7ccafdde727caed5ed7329d29c - many thanks to max for merging for me.) It contains an extra chapter covering annotations, the GHC API, and the compiler plugins API (I moved the old annotations section here.) I just added a new chapter after the FFI chapter. I have forgotten to update the NewPlugins page it seems. I'll do so accordingly this morning.

I also neglected to mention home package plugins in the user manual. I can add these additions later tonight, but someone else will need to merge them for me when the time comes.

RE: testsuite, I believe Max was holding off on the issue of linking getting very weird on windows. Max, can you give us confirmation on where you're at? IIRC, Simon Marlow gave an OK on the patches to not export SRT symbols from object files, which brings the symbol count for libghc just under ~65k exported symbols (which the linker can correctly handle.)

These SRT-export patches have been committed by Max if memory serves correctly. However, it seems that the commit which will fix the actual linking issues for plugins06 (by adding thisGhcPackageId to the linker's init_pkgs, so it does not try to load a new copy of GHCi state) has not been committed yet. This is all that's left to be done. Max attached it in this ticket I believe.

Max - is there anything holding you off from the final linker change? I believe this is all that's necessary to make plugins06 passing. Waiting by for conformation.

comment:50 Changed 3 years ago by batterseapower

I can't commit the patch above until:

  1. Someone can help me with my questions about the build system above
  1. I find time to check that annrun01 failure on Linux

comment:51 Changed 3 years ago by igloo

I can only see one build system question, about the library suffix. Is that what you mean?

The library suffix is $($(way)_libsuf) (e.g. $(v_libsuf) is .a, $(p_libsuf) is _p.a).

So for example in build-package-way.mk we define

$1_$2_$3_LIB = $1/$2/build/libHS$$($1_PACKAGE)-$$($1_$2_VERSION)$$($3_libsuf)

Does that answer the question?

comment:52 Changed 3 years ago by batterseapower

Ian: thanks. The other question is from this point:

  • There is some more opportunity to cut down on the number of exports from ghc-stage2.exe. I don't currently prevent libHSghc-prim, libHSbinary and the RTS from being exported from ghc.exe, because they are only depended on transitively. How can I get hold of their version numbers in ghc.mk?

How do I got hold of the (version numbers/package names including the version suffix) of ghc-prim and binary from within ghc.mk?

comment:53 Changed 3 years ago by igloo

Hmm, we don't currently know the transitive deps, AFAIK. Does it matter if you also get some packages that aren't dependencies of ghc?

If not, you can use this:

$(foreach p,$(PACKAGES),$p-$(libraries/$p_dist-install_VERSION))

comment:54 Changed 3 years ago by batterseapower

  • Status changed from patch to merge

I've worked around the linking issue for now by adding a function CoreMonad?.reinitializeGlobals. A plugin can call this as the first thing it does in order to avoid the static flags issue and others. This function just ensures that the global variables in the new copy of GHC get contents equal to that in the original copy.

When we have the linking issue resolved we can redefine reinitializeGlobals to (return ()) and deprecate it.

The relevant commit is 0e765db44229aed591f9f423ef909b5f76696de0. I've commited a testsuite change marking plugins06 as passing in a67505b6cb4edc53066c6781ffbbf32c2a3bd073.

I've created #5355 to track work on reusing the existing copy of libHSghc rather than linking a new one, because this ticket is getting huge and has lots of irrelevant information.

Ian: is it possible to merge 0e765db44229aed591f9f423ef909b5f76696de0 to 7.2? The plugin functionality would be much more usable in 7.2 with this change.

comment:55 Changed 3 years ago by thoughtpolice

Sorry for making this ticket bigger, but I've also added documentation to the users guide for reinitializeGlobals on my ghc branch here:

https://github.com/thoughtpolice/ghc/tree/plugin-docs-reinit-globals

Relevant commit:

https://github.com/thoughtpolice/ghc/commit/2419ff534ea8d30febb69a05fb9026d0d4473694

Max, Ian: if one of you could at least merge this to master, that would be awesome. If Max's reinitializeGlobals change makes it into the 7.2 branch, I'd highly advise this be merged to so people can be aware of this issue.

comment:56 Changed 3 years ago by simonmar

Max: you mentioned there was some code size impact from these changes. Is that due to the extra entries in the dynamic symbol table, or something else?

comment:57 Changed 3 years ago by batterseapower

Simon: I would have to check. I originally assumed that it forces the linker to link code from libHSghc into the executable that it could otherwise have dropped as dead code, but it could also have been from raw symbol table size too.

On linux I saw an increase to 47mb from 32mb, i.e. 15mb. If the dynamic symbol table exported 65536 symbols (roughly right) then the entire size increase would be explained if the average symbol length was 232 bytes (assuming 8 bytes overhead per symbol in the table to store an address). This does not seem at all unreasonable, as the symbols GHC generates are rather long.

So it seems quite possible that the majority of the size increase comes purely from the dynamic symbol table.

(Note that the patch that causes code size bloat has *not* been committed)

comment:58 Changed 3 years ago by igloo

  • Owner simonpj deleted
  • Status changed from merge to new

changeset:0e765db44229aed591f9f423ef909b5f76696de0 merged as changeset:59862080d2abe6fa0f8f0e3e46391f54d4f76e84

The doc patch still needs to be applied. After that, please close this ticket and open a new one for any other related issues.

comment:59 Changed 3 years ago by igloo

  • Status changed from new to patch

comment:60 Changed 3 years ago by batterseapower

  • Resolution set to fixed
  • Status changed from patch to closed

Merged Austin's patch to master as af0a6aae3cc6ccc13319a9fbe0076e65374e9fe7 and to ghc-7.2 as 535e8f50adb7fc0ed2bf9e225840012576a37f35

Note: See TracTickets for help on using tickets.