WIP: Make SpecConstr work across modules
Needs ReviewPublic

Authored by mpickering on May 10 2017, 9:53 AM.

Details

Reviewers
simonpj
bgamari
austin
Trac Issues
#10346
Summary

This enables the SpecConst transformation to work
across modules. I mostly copied and modified code from the normal specialiser
and it seems to work. Here to validate and get feedback.

TODO:

  • Work out what SpecConstr actually does
  • Add a test
  • Clean up the mostly copied implementation
mpickering created this revision.May 10 2017, 9:53 AM
mpickering edited the summary of this revision. (Show Details)May 10 2017, 9:55 AM
mpickering updated this revision to Diff 12499.May 11 2017, 5:43 AM
  • Remove traces
dfeuer added a subscriber: dfeuer.May 16 2017, 1:55 AM
dfeuer added inline comments.
compiler/specialise/SpecConstr.hs
719

Reversing isn't free. Do you really have to do it twice here?

744

There's an awful lot of very lazy-looking stuff going on in goEnv. Could you maybe add a comment explaining why we want that, or alternatively why it's less lazy than it looks? Or just remove it:

let !(env', bind') = ...
    !(env'', binds') = ...
in ...
dfeuer added inline comments.May 16 2017, 2:01 AM
compiler/specialise/SpecConstr.hs
768

This do is currently redundant.

1520

Is there a reason for all the lazy pattern matches in scTopBindEnv?

Thanks for looking David but I'm not sure this patch works properly yet at all. Do you have an idea of a small test I could add?

Matthew, I have not done a detailed review, but it looks as if you are following the pattern in Specialise.hs. Can you write an overview Note explaining the goal, with an example, and pointing to where things happen.

compiler/specialise/SpecConstr.hs
1877

Why wasn't this necessary before?

bgamari requested changes to this revision.Jun 5 2017, 10:33 AM

Bumping out of the review queue for now.

This revision now requires changes to proceed.Jun 5 2017, 10:33 AM
austin resigned from this revision.Nov 9 2017, 11:38 AM