Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed some major issues related to Dead Variable Elimination #23

Merged
merged 22 commits into from May 30, 2019

Conversation

Anabra
Copy link
Member

@Anabra Anabra commented Feb 11, 2019

The problem

Dead Variable Elimination treated side-effecting computation incorrectly.
Take a look at the following example:

x <- pure (CInt 5)
y <- case x of
  (CInt n) ->
    _prim_int_print 5
    pure 5
pure 0

The variable y is DEAD, because it is irrelevant in the context of the pure result of the program. However, its binding cannot be removed, because it contains a side-effecting computation. If we were to remove this, the semantics of the program would change.

The solution

Treat side-effecting computations in the transformation level using EffectMap. Now Dead Variable Elimination will perform a simple bottom-up analysis of the syntax tree, and collect so-called "protected" variables. These variables are protected, because some expression with a side-effecting computation is bound to them. Protected variables cannot be removed from the program.

Note that this analysis is local, it will not follow function calls.

Prerequisites

Pattern bindings need special treatment, because their left-hand side can contain any arbitrary expression. This makes it hard to "identify" a computation. To resolve this problem with minimal effort, we introduce a new convention: Each pattern binding with a non-variable pattern must have a simple left-hand side, that is they can only be of the form: pure <var>. Also, for convenience, all case scrutinees can only be variables as well. This ensures that every "real" computation has a name.

To introduce these conventions, we define a new transformation: BindingPatternSimplification.

Alternative solution

We could implement an interprocedural analysis which calculates the possible side-effecting computations for every function and binding. Currently, EffectMap only calculates this information for functions, which is not sufficient.

Failable patterns

Case expressions containing failable patterns can give rise to a very similar problem to side-effecting computations. See the code below.

x <- pure (CInt 5)
y <- case x of
  (CUnit) -> pure 5
pure 0

The variable y is DEAD considering the pure result of the program, but a failable case expression is bound to it. If we consider pattern match errors to be implicit (i.e.: the runtime would throw an error whenever it cannot match a value on any pattern), then this is a very similar problem to the one mentioned above. However, if pattern match errors are explicit (i.e.: there is always a #default alternative with an error throwing right-hand side), then the above piece of code has undefined semantics, and we do not have to consider it for any analysis or transformation.

This question requires further discussion.

@coveralls
Copy link

coveralls commented Feb 11, 2019

Coverage Status

Coverage increased (+0.4%) to 51.551% when pulling 8d72e0e on Anabra:master into 5a674ef on grin-tech:master.

grin/app/GrinCLI.hs Outdated Show resolved Hide resolved
Anabra added 13 commits May 19, 2019 21:20
Copy propagation now no longer viloates PNI conventions
(disabled right-unit law)

Added a new ExpChange type: DeletedHeapOperation
Added DeletedHeapOperation effect to NonSharedElimination
Now NonsharedElimination only invalidates the analysis results
if it actually changed the AST.
When binding into a variable, both liveness and strutural information
was propagated both ways. Now liveness info is only propagated backward
and structural info is only propagated forward.
Now DVE correctly handles case expressions with side-effecting
computations.

Now DVE correctly handles case expressions with failable
patterns.
Live Variable Anaylsis now only marks node tags live
if they are either bound out to a node pattern,
or pattern matched on AND the result of the expression
is live.

Side-effects and pattern match failure checks are handled by DVE.
BPS will make DVE much simpler to implement
BPS makes sure that:
  - only variable can be bound to non-variable patterns
  - all case scrutinees are variables
Now DVE correctly handles binding patterns.
DVE now correctly handles store/fetch pairs in dead alternatives
notes/tests.todo Outdated Show resolved Hide resolved
grin/src/Transformations/Optimising/CopyPropagation.hs Outdated Show resolved Hide resolved
grin/test-data/experimental/dve/bug_1.grin Outdated Show resolved Hide resolved
grin/app/GrinCLI.hs Outdated Show resolved Hide resolved
grin/src/Pipeline/Pipeline.hs Outdated Show resolved Hide resolved
grin/src/Grin/Pretty.hs Outdated Show resolved Hide resolved
grin/src/Pipeline/Pipeline.hs Outdated Show resolved Hide resolved
grin/src/Pipeline/Pipeline.hs Outdated Show resolved Hide resolved
Right unit law still works, but the last expression
in a binding sequence always must be of form "pure <var>".
Rendering option for pretty printing was a global option for the pipeline.
Now it is an argument for the printer flag.
Removed unused function.
notes/tests.todo Outdated Show resolved Hide resolved
grin/app/GrinCLI.hs Outdated Show resolved Hide resolved
grin/test-data/.gitignore Outdated Show resolved Hide resolved
Removed some dead code
Added newlines
@csabahruska csabahruska merged commit dd219c7 into grin-compiler:master May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants