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

[mlir] move alias lattice update to relevant transfer function #1573

Merged
merged 6 commits into from
Feb 27, 2024

Conversation

ftynse
Copy link
Collaborator

@ftynse ftynse commented Dec 7, 2023

Although the framework allows a transfer function of one analysis to update a lattice associated with another analysis as long as it triggers propagation correctly, this is difficult to reason about.

This requires injecting ModRef information about library functions.
Historically, `AliasClassSet` has been using empty set as the bottom
state of the join-semilattice. This made it impossible to differentiate
between "not yet analyzed" and "known not to alias with anything"
states. While not necessarily problematic for the naive alias analysis,
this is crucial for points-to-pointer analysis that was forced to become
unnecessarily conservative by treating "not yet analyzed" points-to
state as "points to unknown" (top sate of the join-semilattice). Worse,
some transfer functions weren't trivially monotonous because of that.

Introduce a new "undefined" set to `AliasClassSet` and lattice. Update
PointsToLattice to explicitly store the cases when a known alias class
points to unknown alias class and interpret unknown (not yet listed)
alias class as pointing to undefined alias class. Make all transfer
functions monotonous by only processing known points-to entries. Unknown
points-to classes can be treated by the client as potentially pointing
to unknown, but so far this wasn't needed as, at the fixpoint state, all
available classes should have been analyzed. Assertions are put in place
in activity analysis to catch undefined classes.

Additionally, make it more difficult to move the join-semilattice to a
previous (<= current) value by exposing mostly the APIs that can only
advance the lattice. This is not yet complete due to partially incorect
handling of distinct identifers that get recreated every time in
transfer functions.
Previously, dataflow initialization and transfer functions would
allocate a fresh alias class for allocation-like cases that are known
not to alias with anything, and do so _on every invocation_. This is
unsound in the general case as the class may keep changing, which could
affect convergence to fix point. Instead, add a cache that associates a
class with a value on the first invocation of the dataflow-related
functions and use the cached value on each subsequent invocation.

Fix a couple of issues unconvered due to the change:
  - not recreate the common scope that function results may be pointing
    to for each capturable operand;
  - invert the condition of handling nocapture in function results;
  - add a TODO for better handling of alias classes of function results
    in presence of function-level attributes.
Specifically, handle the siutation where some alias classes are
associateded with pointer operands that cannot be written into. In such
a case, even if the results of the function may alias the operands, the
alias classes of the operands are known not to point to anything that
the function could have written.

Additionally, don't include results marked as `noalias` into the common
alias class of function results.
Although the framework allows a transfer function of one analysis to
update a lattice associated with another analysis as long as it triggers
propagation correctly, this is difficult to reason about.
Base automatically changed from ftynse/undefined to main January 18, 2024 16:28
@wsmoses wsmoses marked this pull request as ready for review January 18, 2024 16:28
@wsmoses wsmoses merged commit f7a46fd into main Feb 27, 2024
41 of 47 checks passed
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.

3 participants