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

Add F_POINTED_AT; seems to work after some minimal testing #298

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

eliotmoss
Copy link
Contributor

No description provided.

Copy link
Owner

@titzer titzer left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -329,7 +329,8 @@ class ReachabilityNormalizer(config: NormalizerConfig, ra: ReachabilityAnalyzer)
// add normalized field(s) to the vector
var tn = rf.typeNorm;
var facts = if(tn.size > 1, Fact.F_NORM, Facts.NONE);
if (!rf.raFacts.RF_WRITTEN) facts |= (Fact.F_VALUE | Fact.O_FOLDABLE);
if (!rf.raFacts.RF_WRITTEN && !rf.orig.facts.F_POINTED_AT) facts |= (Fact.F_VALUE | Fact.O_FOLDABLE);
Copy link
Owner

Choose a reason for hiding this comment

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

Note I added this line while resolving a conflict here.

@titzer
Copy link
Owner

titzer commented Nov 18, 2024

I've confirmed the CI fails because the additional fact brought the size of a Fact.set over 32-bit, which seems to have exposed some bugs in unboxing (that's why this only fails on the CI configurations that try to unbox all variants). I'll try to get a repro and fix those other bugs. In the mean time, can you remove the unused O_ASSOCIATIVE case?

@eliotmoss
Copy link
Contributor Author

eliotmoss commented Nov 18, 2024 via email

@titzer
Copy link
Owner

titzer commented Nov 19, 2024

Yes - but do you mean in the PR or only locally? Eliot

In the PR. Since it only exposed a pre-existing bug (which I have a fix for, but not yet a smaller repro), no need to hold up this PR.

@eliotmoss
Copy link
Contributor Author

eliotmoss commented Nov 19, 2024 via email

@titzer titzer merged commit ed7f584 into titzer:master Nov 19, 2024
10 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.

2 participants