Reorganize opcodes by kind and type #48
Conversation
| | `i16x8.all_true` | `0x6e`| - | | ||
| | `i32x4.add` | `0x6f`| - | | ||
| | - | `0x70`| - | | ||
| | - | `0x71`| - | |
There was a problem hiding this comment.
Reserved for saturating adds.
There was a problem hiding this comment.
There are no hardware instructions that map to saturating arithmetic for i32x4, and i64x2 types that I know of, so I'm unsure if this will ever be useful - but the opcode space is large enough that this looks ok to me. Same for the other saturating operations below.
| | `i16x8.sub_saturate_u` | `0x65`| - | | ||
| | `i16x8.mul` | `0x66`| - | | ||
| | - | `0x67`| - | | ||
| | - | `0x68`| - | |
| | `i8x16.sub_saturate_u` | `0x56`| - | | ||
| | `i8x16.mul` | `0x57`| - | | ||
| | - | `0x58`| - | | ||
| | - | `0x59`| - | |
proposals/simd/BinarySIMD.md
Outdated
| | - | `0x41`| - | | ||
| | - | `0x42`| - | | ||
| | - | `0x43`| - | | ||
| | - | `0x44`| - | |
There was a problem hiding this comment.
Reserved space for all possible i64x2 comparisons.
| | - | `0x71`| - | | ||
| | `i32x4.sub` | `0x72`| - | | ||
| | - | `0x73`| - | | ||
| | - | `0x74`| - | |
There was a problem hiding this comment.
Reserved for saturating subtractions.
| | - | `0x74`| - | | ||
| | `i32x4.mul` | `0x75`| - | | ||
| | - | `0x76`| - | | ||
| | - | `0x77`| - | |
| | `i32x4.all_true` | `0x7d`| - | | ||
| | `i64x2.add` | `0x7e`| - | | ||
| | - | `0x7f`| - | | ||
| | - | `0x80`| - | |
There was a problem hiding this comment.
Reserved for saturating adds.
| | - | `0x83`| - | | ||
| | - | `0x84`| - | | ||
| | - | `0x85`| - | | ||
| | - | `0x86`| - | |
There was a problem hiding this comment.
Reserved for saturating subtractions, multiply, and min/max.
|
@tlively Are you planning to do any further changes to the opcode sequence? Its better to combine all reorganizations that seem fit as a single PR. This is to make it easier for the toolchain and implementations to incorporate the changes. |
|
@aretmr No, I don't plan further changes. This PR is an attempt to combine and finalize all of the reorganizations that are currently in flight. If anyone has any other ideas for reorganization, we should discuss and incorporate them before merging so we can do just one change to bring everything into alignment everywhere. |
proposals/simd/BinarySIMD.md
Outdated
| | `f64x2.splat` | `0x15`| - | | ||
| | `f64x2.extract_lane` | `0x16`| i:LaneIdx2 | | ||
| | `f64x2.replace_lane` | `0x17`| i:LaneIdx2 | | ||
| | `v128.and` | `0x18`| - | |
There was a problem hiding this comment.
These should be moved to after the comparisons.
| | `i16x8.all_true` | `0x6e`| - | | ||
| | `i32x4.add` | `0x6f`| - | | ||
| | - | `0x70`| - | | ||
| | - | `0x71`| - | |
There was a problem hiding this comment.
There are no hardware instructions that map to saturating arithmetic for i32x4, and i64x2 types that I know of, so I'm unsure if this will ever be useful - but the opcode space is large enough that this looks ok to me. Same for the other saturating operations below.
| | `i8x16.shr_s` | `0x5b`| - | | ||
| | `i8x16.shr_u` | `0x5c`| - | | ||
| | `i8x16.neg` | `0x5d`| - | | ||
| | `i8x16.any_true` | `0x5e`| - | |
There was a problem hiding this comment.
It makes sense to me to consistently adopt use v8x16 for sign agnostic operations, it's a little weird to me that we do this only for v8x16.shuffle. Thoughts?
There was a problem hiding this comment.
Also think that using v8x16 for only shuffles is weird (see #38), but I think keeping i8x16 and friends for sign-agnostic operations that still only make sense for integer lanes is reasonable. To counter, what do you think of changing shuffle to be v128.shuffle?
There was a problem hiding this comment.
Using v8x16.shuffle makes sense because it's a generic byte shuffle, so the name is more indicative than a v128.shuffle, but I don't feel strongly about which way we go here.
| | `i8x16.shl` | `0x5a`| - | | ||
| | `i8x16.shr_s` | `0x5b`| - | | ||
| | `i8x16.shr_u` | `0x5c`| - | | ||
| | `i8x16.neg` | `0x5d`| - | |
There was a problem hiding this comment.
Just noticed this while updating wabt: the MVP instructions group unary and binary ops together by type, but have unary before binary, e.g.:
| opcode | name |
|---|---|
0x67 |
i32.clz |
0x68 |
i32.ctz |
0x69 |
i32.popcnt |
0x6a |
i32.add |
| ... |
| | `f32x4.div` | `0x90`| - | | ||
| | `f32x4.min` | `0x91`| - | | ||
| | `f32x4.max` | `0x92`| - | | ||
| | `f32x4.neg` | `0x93`| - | |
There was a problem hiding this comment.
The MVP instructions have the order abs, neg, sqrt
|
@binji, I notice the MVP instructions have |
This is yet another opcode renumbering.
I tried to take into account all the comments on #42, particularly @binji's comment about trying to be consistent with the MVP opcode ordering. I also left explicit gaps in the chart for what I think should be reserved opcodes to make the reserved opcodes easier to see and talk about for the time being.
Feedback welcome!
cc @dtig