Stream: git-wasmtime

Topic: wasmtime / PR #5821 Fix the postorder traversal in the Do...

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:26):

elliottt opened PR #5821 from trevor/fix-postorder-traversal to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:31):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:31):

bjorn3 created PR review comment:

The stack is not stored in rpo_number.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:32):

elliottt edited PR #5821 from trevor/fix-postorder-traversal to main:

Fix the postorder traversal computed by the DominatorTree. It was recording nodes in the wrong order depending on the order child nodes were visited. Consider the following program:

function %foo2(i8) -> i8 {
block0(v0: i8):
    brif v0, block1, block2

block1:
    return v0

block2:
    jump block1
}

The postorder produced by the previous implementation was:

block2
block1
block0

Which is incorrect, as block1 is branched to by block2. Changing the branch order in the function would also change the postorder result, yielding the expected order with block1 emitted first.

This PR reworks the implementation of DominatorTree::compute to produce an order where block1 is always returned first, regardless of the branch order in the original program.

Co-authored-by: Jamey Sharp <jsharp@fastly.com>

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:35):

elliottt updated PR #5821 from trevor/fix-postorder-traversal to main.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:38):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:38):

elliottt created PR review comment:

We use the SEEN value to imply something about the state of the stack, we're not trying to suggest that the stack is stored in rpo_number. Is there a way that this can be rephrased to make this more clear?

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:41):

elliottt updated PR #5821 from trevor/fix-postorder-traversal to main.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:41):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:41):

bjorn3 created PR review comment:

Missed the line break in this sentence. I thought it was a separate thing. My bad.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:42):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:42):

elliottt created PR review comment:

Ah, that makes sense! Would indenting it a little more make it read better?

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:44):

elliottt requested cfallin for a review on PR #5821.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:53):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 19:53):

bjorn3 created PR review comment:

That could help.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:02):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:04):

elliottt updated PR #5821 from trevor/fix-postorder-traversal to main.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:11):

elliottt has enabled auto merge for PR #5821.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:12):

elliottt has disabled auto merge for PR #5821.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:17):

elliottt updated PR #5821 from trevor/fix-postorder-traversal to main.

Wasmtime GitHub notifications bot (Feb 17 2023 at 20:18):

elliottt has enabled auto merge for PR #5821.

Wasmtime GitHub notifications bot (Feb 17 2023 at 21:23):

elliottt merged PR #5821.

Last updated: Nov 22 2024 at 17:03 UTC