Stream: git-wasmtime

Topic: wasmtime / PR #5113 Cranelift: Avoid calling `ensure_stru...

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:24):

fitzgen opened PR #5113 from avoid-cloning-sigs to main:

Unfortunately, this seems to be perf neutral. I think we already got most of the wins we otherwise would have got when we deduplicated IR signatures during lowering.

<!--

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 (Oct 24 2022 at 22:24):

fitzgen requested cfallin for a review on PR #5113.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:40):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:40):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:40):

jameysharp created PR review comment:

I haven't looked at the implementation of uses_special_return. Why does it need to be guarded to only run when there are some return values?

Maybe the answer is just that the old implementation had this guard. But I think the old implementation, using Iterator::any, shouldn't have needed this either: any returns false on an empty iterator.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:40):

jameysharp created PR review comment:

/// If the signature needs to be legalized, then return the struct-return

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:44):

fitzgen created PR review comment:

It isn't that uses_special_return forces this, it is the pre-existing logic of when we need to legalize, and I have no idea why it is like that, I just faithfully kept it the same.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:44):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:44):

fitzgen updated PR #5113 from avoid-cloning-sigs to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:46):

fitzgen updated PR #5113 from avoid-cloning-sigs to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 23:38):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 23:38):

cfallin created PR review comment:

Yeah, it looks like it was an over-eager guard that wasn't actually necessary. Latest version looks good.

Wasmtime GitHub notifications bot (Oct 24 2022 at 23:38):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Oct 25 2022 at 00:21):

fitzgen merged PR #5113.

Last updated: Dec 23 2024 at 12:05 UTC