Skip to content

Use exactness to optimize GUFA cone depth (2nd alternative) #7582

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

Merged
merged 14 commits into from
May 13, 2025

Conversation

tlively
Copy link
Member

@tlively tlively commented May 10, 2025

Exact references are known to point to exactly their heap type and not
one of its subtypes. GUFA already analyzed exactness via the more
general "cone" types that include an allowed subtyping depth. Exact
types are equivalent to cone types of depth zero. Let GUFA take
advantage of exactness information by normalizing cone depths to 0
whenever the cone type is exact.

@tlively tlively requested a review from kripken May 10, 2025 03:06
@tlively tlively changed the title Use exactness to optimize GUFA cone depth Use exactness to optimize GUFA cone depth (2nd alternative) May 10, 2025
@tlively
Copy link
Member Author

tlively commented May 10, 2025

This is an alternative to #7538 based on the feedback from #7538 (comment). See commit cff042c for the diff from #7538 at time of writing.

I don't feel strongly one way or another between the two approaches, but this one has to touch more code. It's unclear to me whether there is a functional difference between the two, although they pass the same tests.

After having wrestled with this code for a while on multiple occasions, I'm sorely tempted to jump in and rewrite PossibleContents as a composition of lattices.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think this is better! Removing fullConeType() makes things much clearer.

@@ -371,7 +357,18 @@ bool PossibleContents::isSubContents(const PossibleContents& a,
return false;
}

WASM_UNREACHABLE("unhandled case of isSubContents");
if (b.isGlobal()) {
// We've already ruled out anything but another global or cone type for a.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We've already ruled out anything but another global or cone type for a.
// We've already ruled out anything but another global or (non-full) cone type for a.

@@ -128,7 +128,9 @@ class PossibleContents {

// Internal convenience for creating a cone type of unbounded depth, i.e., the
// full cone of all subtypes for that type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment needs updating.

@@ -150,8 +152,8 @@ class PossibleContents {
}
// Helper for a cone with unbounded depth, i.e., the full cone of all subtypes
// for that type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@kripken
Copy link
Member

kripken commented May 12, 2025

I'm sorely tempted to jump in and rewrite PossibleContents as a composition of lattices.

I'd be curious to see what that looks like. Do you mean with the existing Lattice?

@tlively
Copy link
Member Author

tlively commented May 13, 2025

I'm sorely tempted to jump in and rewrite PossibleContents as a composition of lattices.

I'd be curious to see what that looks like. Do you mean with the existing Lattice?

Yes, exactly. The only missing piece preventing this from being a quick piece of work is a general Abstraction lattice whose elements are a variant of increasingly abstract sub-lattices. I would take a day to work on this, but finishing the custom descriptors and shared-everything features is becoming increasingly urgent.

@tlively tlively merged commit 1f27e2f into main May 13, 2025
14 checks passed
@tlively tlively deleted the gufa-exact-cone-alt branch May 13, 2025 17:17
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.

2 participants