Stream: git-wasmtime

Topic: wasmtime / PR #1992 Refactor the InstSize enum in the AAr...

Wasmtime GitHub notifications bot (Jul 08 2020 at 11:13):

akirilov-arm opened PR #1992 from InstSize to main:

As discussed in #1748, the main issue with the InstSize enum was that it was used both for GPR and SIMD & FP operands, even though machine instructions do not mix them in general (as in a destination register is either a GPR or not). As a result it had methods such as sf_bit() that made sense only for one type of operand.

Another issue was that the enum name was not reflecting its purpose accurately - it was meant to represent an instruction operand size, not an instruction size, which is fixed in A64 (always 4 bytes).

Now the enum is split into one for GPR operands and another for scalar SIMD & FP operands.

Wasmtime GitHub notifications bot (Jul 08 2020 at 11:45):

akirilov-arm updated PR #1992 from InstSize to main:

As discussed in #1748, the main issue with the InstSize enum was that it was used both for GPR and SIMD & FP operands, even though machine instructions do not mix them in general (as in a destination register is either a GPR or not). As a result it had methods such as sf_bit() that made sense only for one type of operand.

Another issue was that the enum name was not reflecting its purpose accurately - it was meant to represent an instruction operand size, not an instruction size, which is fixed in A64 (always 4 bytes).

Now the enum is split into one for GPR operands and another for scalar SIMD & FP operands.

Wasmtime GitHub notifications bot (Jul 13 2020 at 17:52):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jul 13 2020 at 17:52):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jul 13 2020 at 17:52):

cfallin created PR Review Comment:

Perhaps use a match here, and panic if bits isn't exactly 8 / 16 / 32 / 64 / 128? I'm not sure what it would mean to have (e.g.) a 24-bit scalar...

Wasmtime GitHub notifications bot (Jul 13 2020 at 17:52):

cfallin created PR Review Comment:

Doc-comment here indicating that it returns encoding bits

Wasmtime GitHub notifications bot (Jul 14 2020 at 14:18):

akirilov-arm updated PR #1992 from InstSize to main:

As discussed in #1748, the main issue with the InstSize enum was that it was used both for GPR and SIMD & FP operands, even though machine instructions do not mix them in general (as in a destination register is either a GPR or not). As a result it had methods such as sf_bit() that made sense only for one type of operand.

Another issue was that the enum name was not reflecting its purpose accurately - it was meant to represent an instruction operand size, not an instruction size, which is fixed in A64 (always 4 bytes).

Now the enum is split into one for GPR operands and another for scalar SIMD & FP operands.

Wasmtime GitHub notifications bot (Jul 14 2020 at 14:20):

akirilov-arm submitted PR Review.

Wasmtime GitHub notifications bot (Jul 14 2020 at 14:20):

akirilov-arm created PR Review Comment:

This just mirrors the original logic in InstSize. Anyway, I figured out a way to express it succinctly.

Wasmtime GitHub notifications bot (Jul 14 2020 at 15:58):

cfallin merged PR #1992.

Last updated: Nov 22 2024 at 17:03 UTC