-
Notifications
You must be signed in to change notification settings - Fork 186
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
4 changed files
with
171 additions
and
51 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
361252d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm struggling to follow all your commits :-).
I suspect you do not deal correctly with partial application: if you have a function with two parameters and apply it to only one argument, then you don't know what the second argument will be bound to.
It seems to me that with your changes,
get_approx
duplicates somewhat what is done insolver1
.So, it seems to me it would be much better to refine the analyses:
propagate1
should return the set of known arguments when applied to a function parameter; when applied toExpr (Apply _)
, it should record the dependency between the arguments and the parameters (both in a table of typeVarSet.t VarTbl.t
and by callingadd_dep
;propagate2
should be modified as well: the value of a parameter may be unknown if the function escapes, one of the corresponding arguments may be unknown, or the function is partially applied (and there is no argument corresponding to this parameter).But we then have a problem with
build_subst
, which cannot any longer rely on these analyses. In particular, we cannot substitute variable across function boundaries : in the piece of code below, you cannot replacex
byz
which is not in scope:So, I think we need a specific analysis for variable renaming. The way this is implemented at the moment is a bit of a hack anyway. This is related to computing dominators (see "A Simple, Fast Dominance Algorithm"): basically, what we want is to replace each variable by the root of the corresponding dominator tree. I need to think about how to compute that.
So, I think the first step is to change
build_subst
not to depend on previous analyses, and then we can improve these analyses.