Skip to content

Conversation

@mayanje
Copy link
Contributor

@mayanje mayanje commented Jul 23, 2021

What has been done
Basic dominator algorithms have been added.

Possible Improvement
Later use faster dominator algorithm, but much more complicate to develop and to understand.

@mayanje mayanje added this to the v1.6 milestone Jul 23, 2021
@mayanje mayanje requested review from fm-117 and rooksdo July 23, 2021 16:10
@mayanje mayanje self-assigned this Jul 23, 2021
@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Jul 23, 2021
if (n == this.RootBlock.Index)
continue;
BasicBlock<N, D> b = this.AllBlocks[n];
if (b.PredecessorEdges == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it a Debug.Assert instead ? We ensure that predecessor edges are set with previous call to SetupPredecessorEdgesFromRoot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in
ff21062

BitSet doms = dominators[i];
if (doms != null)
{
BasicBlock<N, D> b = this.AllBlocks[i];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is b.Index not always equal to i ? Intuitively I thought that this method should be static because it does not depend on the current CFG (except if we include other info from the block).
Also why the double braces ? Is it a common notation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes done in
d9ea371

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and
9dcd9e1 double bracket removed

@trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jul 26, 2021
@trafico-bot trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Jul 26, 2021
@mayanje mayanje requested a review from fm-117 July 26, 2021 17:46
@trafico-bot trafico-bot bot removed the 🔍 Ready for Review Pull Request is not reviewed yet label Jul 27, 2021
@mayanje mayanje removed the request for review from rooksdo July 27, 2021 13:13
@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Jul 27, 2021
@mayanje mayanje merged commit e0d65ad into develop Jul 28, 2021
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jul 28, 2021
@mayanje mayanje mentioned this pull request Oct 19, 2021
@fm-117 fm-117 deleted the 2014_CFG_Implement_Dominator branch November 18, 2022 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DataFlow ✨ Merged Pull Request has been merged successfully TypeCobol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants