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 dynamic pure loop cutting #619

Merged
merged 3 commits into from
Feb 25, 2024
Merged

Conversation

ThomasHaas
Copy link
Collaborator

@ThomasHaas ThomasHaas commented Feb 18, 2024

The goal of this PR is to improve DynamicPureLoopCutting in order to fix issues like #584 and maybe let it detect different types of (conditional) side-effects.(Will be done in a different PR).

I added a new general-purpose DominatorTree class that can compute and represent dominator trees of graphs.
This class might be useful for other program (or even wmm?) analyses.

  • Implemented new DominatorTree class.
  • Simplified DynamicPureLoopCutting using DominatorTree.
  • Rework DynamicPureLoopCutting
  • Merge DynamicPureLoopCutting and SimpleSpinLoopDetection into a single pass (the pass is named DynamicSpinLoopDetection).

@ThomasHaas
Copy link
Collaborator Author

ThomasHaas commented Feb 20, 2024

Issue #584 should be fixed in this PR now. I don't know about issue #583.
I have tested neither of those yet.

The new loop cutting is semantically equivalent to the old one (it detects the exact same sort of side effects).
I will extend it with new features in the future but in a different PR.

EDIT: I think I can now merge DynamicPureLoopCutting with SimpleSpinloopDetection and I will do it in this PR. (DONE)

@hernanponcedeleon
Copy link
Owner

I confirm both issues are solved now.

I still need to review the code.

Comment on lines +86 to +88
// FIXME: The reasoning about safe/unsafe registers is not correct because
// we do not traverse the control-flow but naively go top-down through the loop.
// We need to use proper dominator reasoning!

Choose a reason for hiding this comment

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

We have this DominatorTree class, what else is needed to properly do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nothing but time :P. This code was always like this (i.e. not entirely correct).

Added simple DominatorAnalysis with static helper methods.
Simplified DynamicPureLoopCutting using new dominator trees.
It now instruments the code without using ExecutionStatus.
into a single new pass: DynamicSpinLoopDetection.
- Renamed OptionNames accordingly.
@hernanponcedeleon hernanponcedeleon merged commit bf457e3 into development Feb 25, 2024
1 check passed
@hernanponcedeleon hernanponcedeleon deleted the loopCutting branch February 25, 2024 14:05
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