Skip to content

Fix bug and leak in relooper merge consecutive blocks#2159

Merged
kripken merged 3 commits intoWebAssembly:masterfrom
hobby8:master
Jun 7, 2019
Merged

Fix bug and leak in relooper merge consecutive blocks#2159
kripken merged 3 commits intoWebAssembly:masterfrom
hobby8:master

Conversation

@hobby8
Copy link
Copy Markdown
Contributor

@hobby8 hobby8 commented Jun 1, 2019

Fixes in Relooper merge consecutive blocks:

  1. Entry block getting removed when it is part of a loop:
bb1->AddBranchTo(bb2, nullptr);
bb1->AddBranchTo(bb3, ...);
bb2->AddBranchTo(bb1, nullptr);
bb3->AddBranchTo(bb4, nullptr);
relooper.AddBlock(bb1);
relooper.AddBlock(bb2);
relooper.AddBlock(bb3);
relooper.AddBlock(bb4);
relooper.Calculate(bb1);
  1. Branches memory leak

@kripken
Copy link
Copy Markdown
Member

kripken commented Jun 3, 2019

Thanks @hobby8! This looks right to me.

  1. Have you joined the wasm w3c group?
  2. Please add a test for this. There are a bunch of relooper tests in test/example/ and this can be added to one of them, I think.

@hobby8
Copy link
Copy Markdown
Contributor Author

hobby8 commented Jun 6, 2019

Opted for a minimal new test that exercises mostly the CFG API, hope that's OK.

@hobby8
Copy link
Copy Markdown
Contributor Author

hobby8 commented Jun 7, 2019

I forgot to mention joining the CG. If you prefer to not maintain an additional test, I can look at the existing ones in more detail.

@kripken
Copy link
Copy Markdown
Member

kripken commented Jun 7, 2019

Great, thanks @hobby8! And test looks very good.

@kripken kripken merged commit 5f252c3 into WebAssembly:master Jun 7, 2019
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