Stream: git-wasmtime

Topic: wasmtime / PR #4721 Remove binary_imm64 and int_compare_i...

Wasmtime GitHub notifications bot (Aug 16 2022 at 17:12):

jameysharp opened PR #4721 from remove-imm-insts to main:

I have no idea yet whether this is a good change, and it definitely
should not be merged as-is. The main issues are that there are a bunch
of tests I haven't fixed up, and some optimizations I've removed. Also I
have no performance measurements yet to see what effect this has on
either compile time or generated code quality.

I just wanted to understand what the *_imm instructions were used for,
and trying to grep for them didn't work out well. So I removed them to
let the compiler tell me where they're used instead.

To avoid having to make as many changes, I introduced a InstImmBuilder
trait. It provides methods like iadd_imm which emit the pair of
instructions that legalization would have decomposed that instruction
into eventually anyway.

I have several questions that this may help with answering:

• Is there significant savings, in IR code size and/or compile time,
  from inlining one immediate operand into some instructions at the
  frontend? Or is the widespread frontend use of these instructions just
  for convenience?

• Can simple_preopt cleanly identify arithmetic identities involving
  constants without rewriting to use these combined forms?

• Is there a compile-time cost to simple_preopt rewriting into combined
  forms, only for legalization to rewrite them away again soon after?

<!--

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 (Aug 16 2022 at 20:13):

jameysharp updated PR #4721 from remove-imm-insts to main.

Last updated: Nov 22 2024 at 16:03 UTC