Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Relational: Use same invalidation strategy as base #1646
base: master
Are you sure you want to change the base?
Relational: Use same invalidation strategy as base #1646
Changes from 4 commits
508d7e2
406ad22
563a5b1
79df614
e03a6f7
bb534ff
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this moved out? Base only does it for non-unknown thread functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic of why it is needed is the same here as it is for the case where a known thread is created: One should not go into multi-threaded mode without notifying the privatization that this switch happened to ensure everything that should be published is published.
Maybe base has some reason this can be avoided, but I'm not really sure why that should be the case. We may want to make an issue to investigate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also see the last few comments in #1551
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1551 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one test in this PR doesn't cover this, does it?
I tried the very simple thing of creating a thread from unknown function pointer and that seems to work with base. That's because threadflag analysis emits the
EnterMultiThreaded
event which is handled by base (and relational).The comment for this hack is about the globals values that get used when analyzing the created thread. For an unknown function pointer, there's no thread body to analyze anyway, so it's not clear to me when it would make a difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test covers it in some sense. We mean to invalidate
e
and outputBut this invalidation does not take and we still claim to be able to prove things about
e
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: That case is listed in the linked issue.