Skip to content
This repository has been archived by the owner on Dec 22, 2021. It is now read-only.

Names of load instructions #297

Closed
ngzhian opened this issue Aug 10, 2020 · 6 comments · Fixed by #322
Closed

Names of load instructions #297

ngzhian opened this issue Aug 10, 2020 · 6 comments · Fixed by #322

Comments

@ngzhian
Copy link
Member

ngzhian commented Aug 10, 2020

I would like to discuss the names of load instructions (this came up in #237 and #271).

Currently, we have:

  • v128.load
  • load splats, v8x16.load_splat, v16x8.load_splat, v32x4.load_splat, v64x2.load_splat
  • load extends, i16x8.load8x8_s ,i16x8.load8x8_u ,i32x4.load16x4_s ,i32x4.load16x4_u ,i64x2.load32x2_s ,i64x2.load32x2_u
  • (proposed addition), v128.load32_zero, v128.load64_zero

Some issues:

  1. There is some inconsistency in the prefix, v128 v.s. v8x16 v.s. i8x16.
  2. The suffix (ignoring the sign extension) is also adhoc: in some cases, it has a shape (load8x8_s), in some cases it has the number of bytes loaded (in load32_zero), and sometimes it's just a word (splat).
  3. This also makes writing the formal text a bit more involved since these are all separate syntax, even though they are all loads.

A more consistent naming convention will be:

  • all instructions use v128.load prefix, this is the return type, it makes it clear that all the loads return a v128
  • followed by number of bits to load, similar to MVP (i32.load8_s)
  • followed by the "kind" of load, splat, extend, zero

With this convention, all the loads now look like:

  • v128.load
  • load splats, v128.load8_splat, v128.load16_splat, v128.load32_splat, v128.load64_splat
  • load extends, v128.load64_extend8x8_s, v128.load64_extend8x8_u, v128.load64_extend16x4_s, v128.load64_extend16x4_u, v128.load64_extend32x2_s, v128.load64_extend32x2_s
  • load extends, v128.load8x8_s, v128.load8x8_u, v128.load16x4_s, v128.load16x4_u, v128.load32x2_s, v128.load32x2_s
  • (proposed addition), v128.load32_zero, v128.load64_zero

(The load extends are a bit wordy, maybe we can come up with a better name.)

cc @tlively @Maratyszcza

@binji
Copy link
Member

binji commented Aug 10, 2020

I like that this is more explicit about the load size. It seems like it's a win for the splat/zero cases, but the extend case is less clear, IMO. Not sure quite how to improve, though.

@tlively
Copy link
Member

tlively commented Aug 11, 2020

Could we balance spec simplicity with readability by having the load-extends be v128.load<shape>_extend_{s,u}? Maybe we could say that 32, 64 and 8x8 are all members of some shape abstraction the syntax can use here?

Edit: In more detail, perhaps 32 and 64 could be aliases for the shapes 32x1 and 64x1?

@rossberg
Copy link
Member

In the basic numeric load instructions we don't spell out "extend", so wouldn't the most symmetric names simply be v128.load<shape>_{s,u}?

@ngzhian
Copy link
Member Author

ngzhian commented Aug 11, 2020

Good point, so the names will be:

  • v128.load
  • load splats, v128.load8_splat, v128.load16_splat, v128.load32_splat, v128.load64_splat
  • load extends, v128.load8x8_s, v128.load8x8_u, v128.load16x4_s, v128.load16x4_u, v128.load32x2_s, v128.load32x2_s
  • (proposed addition), v128.load32_zero, v128.load64_zero

@tlively
Copy link
Member

tlively commented Aug 11, 2020

Those look great to me!

@binji
Copy link
Member

binji commented Aug 11, 2020

Yes, these look pretty good to me too. I still think the extend instructions are not super clear, but that's OK. (still not as bad as i64.extend_i32_s vs. i64.extend32_s)

ngzhian added a commit to ngzhian/simd that referenced this issue Aug 25, 2020
Following suggestions in WebAssembly#297, renaming load splats to something more
consistent.
ngzhian added a commit to ngzhian/simd that referenced this issue Aug 31, 2020
The names are still pre WebAssembly#297, once that's finalize I can fix this up
(after the sync WebAssembly#323).

With this change, test/core/run.py passes on all test cases in simd/.
ngzhian added a commit to ngzhian/simd that referenced this issue Sep 4, 2020
Following suggestions in WebAssembly#297, renaming load splats and load extends to
something more consistent.

Fixed WebAssembly#297.
ngzhian added a commit that referenced this issue Sep 8, 2020
Following suggestions in #297, renaming load splats and load extends to
something more consistent.

Fixed #297.
ngzhian added a commit to ngzhian/simd that referenced this issue Dec 17, 2020
Follow up to WebAssembly#297 for renaming the instructions in binary and text
sections in the spec.
tlively added a commit to llvm/llvm-project that referenced this issue Dec 22, 2020
These instructions previously used prefixes like v8x16 to signify that they were
agnostic between float and int interpretations. We renamed these instructions to
remove this form of prefix in WebAssembly/simd#297 and
WebAssembly/simd#316 and this commit brings the names
in LLVM up to date.

Differential Revision: https://reviews.llvm.org/D93722
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants