Skip to content

Commit

Permalink
Merge pull request #367 from MatthewFluet/simplify-types-fix
Browse files Browse the repository at this point in the history
Fix SimplifyTypes SSA optimization pass

`SimplifyTypes` could fail with a `no varInfo property` internal
compiler error.  `SimplifyTypes` transforms each function by visiting
the blocks via `Function.dfs`.  When visiting a statement,
`setVarInfo` is called for the defined variable.  However, after
identifying an unreachable statement (e.g., the `ConApp` of a
`Useless` constructor), `SimplifyTypes` does not visit the remaining
statements of the block.  When continuing with the DFS,
`SimplifyTypes` may subsequently visit a block that use variables that
were defined (but not visited and, therefore, not `setVarInfo`-ed) by
a previously visited block.

This potential error was previously noted (see the comment at the end
of 19b07c0) and has now been observed in the wild (see
MPLLang/mpl#107).

To fix the error, switch from a DFS (on the control-flow graph) to a
dominator tree (depth-first) traversal, where the dominator tree
traversal does not visit any children blocks of an unreachable block.
This addresses the `no varInfo property` internal compiler error,
because any block that uses a variable must be dominated by the block
that defines the variable.

Also refactor the handling of uninhabited types.  Previously, the
`simplifyType` property had the type `Type.t -> Type.t`, where
`Type.unit` was used to signal an uninhabited type.  However, this is
ambiguous, because `Type.unit` *is* an inhabited type.  The refactored
`simplifyTypeOpt` property has the type `Type.t -> Type.t option`,
where `NONE` is used to signal an uninhabited type.  Many subsequent
simplifications of `SimplifyTypes` follow from this refactoring:

 * A `Statement.t` that purports to produce a value of an uninhabited
   type must be unreachable.  This optimization isn't expressible when
   an uninhabited type is signaled by `Type.unit`, because many
   statements may produce a value of `Type.unit`.
 * A block with an argument of an uninhabited type must be
   unreachable.
 * A function with an argument of an unihabited type must be
   unreachable.
 * It is impossible to `ConApp`, `PrimApp`, `Call`, `Raise`, or
   `Return` with a value of an uninhabited type.
 * An array or vector of an uninhabited type must have length 0.
 * `MLton_eq` and `MLton_equal` at a type of cardinality 1 must return
   true.

But, the above does not actually improve effectiveness of the
optimization, because it simply eliminates dead code slight more
aggressively than before.  This dead code was likely to have been
eliminated by `Shrink` and/or `RemoveUnused`.
  • Loading branch information
MatthewFluet committed Feb 14, 2020
2 parents 2521cb3 + 9b95df4 commit ef0f059
Show file tree
Hide file tree
Showing 2 changed files with 264 additions and 242 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.adoc
Expand Up @@ -8,6 +8,9 @@ Here are the changes from version 20180206 to version YYYYMMDD.

=== Details

* 2020-02-14
** Fixed bug in `SimplifyTypes` SSA optimization pass.

* 2020-01-22
** Added expert `-pi-style {default|npi|pic|pie}` and
`-native-pic {false|true}` options, which can be used to override a
Expand All @@ -16,7 +19,7 @@ Here are the changes from version 20180206 to version YYYYMMDD.

* 2020-01-21
** Support parallel build (i.e., `make -j`). This mainly supports
platforms/packagers tht use a parallel `make` by default; it does
platforms/packagers that use a parallel `make` by default; it does
not obtain significant build speedups.

* 2020-01-11
Expand Down

0 comments on commit ef0f059

Please sign in to comment.