Skip to content

Ruby: Split basic blocks around constant conditionals - #11153

Merged
hvitved merged 3 commits into
github:mainfrom
hvitved:ruby/basic-block-at-conditions
Nov 8, 2022
Merged

Ruby: Split basic blocks around constant conditionals#11153
hvitved merged 3 commits into
github:mainfrom
hvitved:ruby/basic-block-at-conditions

Conversation

@hvitved

@hvitvedhvitved commented Nov 8, 2022

Copy link
Copy Markdown
Contributor

In cases such as

ifxoryfooelsebar

we have a CFG that looks like

graph TD;
xvar(x);
yvar(y);
foo(foo);
bar(bar);
orfalse("or [false]");
ortrue("or [true]");
xvar-- false -->yvar;
xvar-- true -->ortrue;
yvar-- false -->orfalse;
yvar-- true -->ortrue;
ortrue-- true -->foo;
orfalse-- false -->bar;
Loading

In order to record the fact that x or ytrue-controls foo (resp. false-controls bar), we need to make sure that a new basic block is started at both foo and bar; that is, we split the existing basic blocks into two.

@hvitvedhvitved added the no-change-note-required This PR does not need a change note label Nov 8, 2022
@hvitved
hvitved marked this pull request as ready for review November 8, 2022 09:37
@hvitved
hvitved requested a review from a team as a code ownerNovember 8, 2022 09:37

@aibaarsaibaars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

if x or y then
foo
else
foo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use a different name in the else clause to make it easier to validate the expected output.

Suggested change
foo
bar

foo
end

if x or y then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also add a test case for and and perhaps not as well.

@hvitved

Copy link
Copy Markdown
ContributorAuthor

@aibaars : Comments addressed, and DCA looks fine.

@aibaarsaibaars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@hvitved
hvitved merged commit edde3de into github:mainNov 8, 2022
@hvitved
hvitved deleted the ruby/basic-block-at-conditions branch November 8, 2022 12:35
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-requiredThis PR does not need a change noteRuby

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@hvitved@aibaars