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

Improve spinloop detection #786

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Conversation

ThomasHaas
Copy link
Collaborator

@ThomasHaas ThomasHaas commented Nov 22, 2024

Using the new tuple support from #775, this PR improves the detection of local side effects in loops.
Rather than treating any assignment to live variables as a side-effect, we now check that the register's values have actually changed. In other words, the check is more semantic now.
This PR also fixes a bug where we failed to detect certain side-effects in cases where the loop only writes to but does not read from a register (only happened in Libvsync benchmarks AFAIK).

I know of at least one verdict that has changed (clh_mutex) but I haven't updated the unit tests yet.
After the CI runs through, I will update the unit tests with the new expectation.

EDIT: Only in the VMM tests we get a different verdict for clh_mutex. This confused me at the beginning, but the reason is that we unroll the code twice whereas we do only once in all other lock benchmarks (for C11, TSO, ARM, and POWER).

@ThomasHaas
Copy link
Collaborator Author

FYI, this is a small precursor of #772.

@hernanponcedeleon hernanponcedeleon merged commit e8622b7 into development Nov 25, 2024
1 check passed
@hernanponcedeleon hernanponcedeleon deleted the updateSpinloopDetection branch November 25, 2024 14:13
CapZTr pushed a commit to CapZTr/Dat3M that referenced this pull request Nov 26, 2024
* Improve DynamicSpinLoopDetection

* Update verdict of VMMLocksTest
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