[dtig]

As discussed in a previous CG meeting, it would be good to discuss what Integer Min/Max operations should look like. This pull request adds lane-wise Integer Min/Max ops similar to the Floating point Min/Max ops. Linked issue to follow up on AIs from CG meetings.

These have native support on most modern architectures.

On Intel:
i32x4.min_s, i16x8.min_s, i8x16.min_s map directly to pmins(d/w/b)
i32x4.max_s, i16x8.max_s, i8x16.max_s map directly to pmaxs(d/w/b)
i32x4.min_u, i16x8.min_u, i8x16.min_u map directly to pminu(d/w/b)
i32x4.max_u, i16x8.max_u, i8x16.max_u map directly to pmaxu(d/w/b)

On ARM Neon:
i32x4.min_s, i16x8.min_s, i8x16.min_s map directly to smin
i32x4.max_s, i16x8.max_s, i8x16.max_s map directly to smax
i32x4.min_u, i16x8.min_u, i8x16.min_u map directly to umin
i32x4.max_u, i16x8.max_u, i8x16.max_u map directly to umax

On ARMv7, these map directly to vmin/vmax instructions.

[stoklund]

LGTM.

Perhaps you could include a rationale for omitting the 64-bit versions?

[jfbastien]

Would be good to also get information on codegen for different ISAs, as well as perf numbers.

[gnzlbg]

Would be good to also get information on codegen for different ISAs, as well as perf numbers.

@dtig these can all be implemented by using the relational operations returning mask and bitselect, such that: min = (a < b).bitselect(a, b) and max = (a > b).bitselect(a, b).

The meeting notes mention:

DG: We want these because pattern matching is fragile, exact numbers are hard to get, these are useful to include just because pattern matching is fragile - encodes performance cliffs.

Maybe someone can ping DG so that they can chime in.

Instead of adding more instructions we could, instead, add a "recommended encoding" for these to some of the other documents, but IMO any code generator worth its salt needs to lower (a < b).bitselect(a, b) optimally anyways, independently of whether we add min or not, because many languages (C++, Rust), support lane-wise relational operations on packed SIMD vectors and offer a select operation on the resulting mask.

The main advantage I see here is that it reduces binary size and encoding/decoding time, but I'd rather add these if this actually becomes an issue in practice, and not as a preventive measure.

nitpick: This is missing the operations for i64x2 for no apparent reason, and it should update the reference.

[sunfishcode]

Would be good to also get information on codegen for different ISAs, as well as perf numbers.

@dtig these can all be implemented by using the relational operations returning mask and bitselect, such that: min = (a < b).bitselect(a, b) and max = (a > b).bitselect(a, b).

The meeting notes mention:

DG: We want these because pattern matching is fragile, exact numbers are hard to get, these are useful to include just because pattern matching is fragile - encodes performance cliffs.

Maybe someone can ping DG so that they can chime in.

Instead of adding more instructions we could, instead, add a "recommended encoding" for these to some of the other documents, but IMO any code generator worth its salt needs to lower (a < b).bitselect(a, b) optimally anyways, independently of whether we add min or not, because many languages (C++, Rust), support lane-wise relational operations on packed SIMD vectors and offer a select operation on the resulting mask.

Some compilers translate from wasm to machine code in a single pass, for fast application startup. Such compilers aren't worth their salt by typical measures, but they're important for WebAssembly. It's possible to implement pattern matching in such compilers, but it adds complexity to what otherwise can be a very simple compiler.

There will always be ambiguity about what things to add as opcodes and what to leave to pattern matching, however when we find ourselves considering documenting "recommended" patterns, and we expect optimizing engines to recognize them, that's a sign that they may be important enough to add as opcodes.

I agree that (a < b).bitselect(a, b) in source languages is important to optimize. If we add an opcode, we can match the pattern in the wasm producer, rather than in the wasm consumer. Adding opcodes is admittedly only a small part of a much larger performance portability story, but I encourage wasm to take such opportunities where it can.

[gnzlbg]

There will always be ambiguity about what things to add as opcodes and what to leave to pattern matching, however when we find ourselves considering documenting "recommended" patterns, and we expect optimizing engines to recognize them, that's a sign that they may be important enough to add as opcodes.

FWIW I think that in this case there isn't that much ambiguity because the consensus seems to be that these (or something like these) should be added at some point, but that it is not worth delaying the MVP for these.

I also don't like the "recommended" patterns situation much. Maybe we should just start a design document for the v2 of the SIMD spec already, and move these there. That way compilers and code generators that want to support these can do so already by targeting that spec.

[tlively]

FWIW I think that in this case there isn't that much ambiguity because the consensus seems to be that these (or something like these) should be added at some point, but that it is not worth delaying the MVP for these.

I don't think adding these would appreciably delay MVP. They are already implemented in V8 and it would not be much effort to add tools support.

I believe the only things we need to do before rebasing and merging this are:

1. Documentation for why i64x2 is omitted (could also be solved by removing all mentions of 64x2 from the proposal)
2. Performance justification for why these instructions should be included (although maybe we don't need solid numbers if it is obvious to everyone that there will be at least some improvement)

[gnzlbg]

Documentation for why i64x2 is omitted (could also be solved by removing all mentions of 64x2 from the proposal)

The 64-bit element versions are probably omitted due to lack of platform support: on x86_64 they require AVX-512 and are not part of NEON.

Performance justification for why these instructions should be included (although maybe we don't need solid numbers if it is obvious to everyone that there will be at least some improvement)

I think that these are useful (*) and supported by most architectures. I think those two evaluation criteria are enough, and I've argued why in the issue #37 .

(*) e.g., to write the min/max elements of two arrays into a third one, to do a min/max horizontal reduction of an array (implementing C++ min_element, max_element, and minmax_element <algorithm>s for integers), etc.

[Maratyszcza]

Emulating unsigned min/max on pre-AVX512 x86 is notoriously inefficient:

• SSE/AVX/AVX2 doesn't include unsigned comparison, but natively supports unsigned maximum. Thus the most efficient way to emulate unsigned comparison is to compare for equality to unsigned maximum.
• SSE/AVX/AVX2 doesn't include an equivalent of v128.bitselect, so it has to be emulated via three instructions (e.g. AND, ANDNOT, XOR, albeit there are other three-instruction options).

Thus, without the integer minimum/maximum instructions in WebAssembly SIMD spec we would have to simulate unsigned minimum/maximum operations at the cost of 5 (!) instructions, including unsigned minimum/maximum instruction (!).

Emulation of signed minimum/maximum is only marginally cheaper: due to the need to lower v128.bitselect into 3 instructions, it takes 4 instructions total to simulate signed minimum/maximum.

[nfrechette]

Can't _mm_blendv_epi8 be used from SSE4.1 for v128.bitselect?

[Maratyszcza]

No, in PBLENDVB (and other blend instructions) input selection is controlled by the sign of elements in the mask, but v128.bitselect input selection is controlled by corresponding bits of the mask.

[dtig]

Rebasing + updating this PR after renewed interest. Updated the PR text to include mapping to ISAs.

These are not included for the 64x2 variants because they are not widely used, as well as the unsigned variants have suboptimal mappings on Intel platforms.

Going back to the discussion about recommended patterns, I think there were a couple of conversations there as the notes don't differentiate between two speakers both with initials DG. I want to clarify the performance example for pattern matching being fragile. The particular example here was about LLVM's current optimizations where in certain cases integer min/max don't lower down exactly to expected nodes even when Min/Max are assumed to be available. So having these operations be explicitly part of the operation set gives applications the flexibility of using them, and not relying on pattern matching when possible.

This specific example was -
uint8x16 _max_u8x16(const uint8x16 a, const uint8x16 b) {
const uint8x16 s1 = (a > b);
return (s1 & a) | (~s1 & b);
}

With saturating add/subtract enabled, the codegen will be lowered to -

42373b: 66 41 0f d8 c7 psubusb %xmm15,%xmm0
423740: 66 0f eb c4 por %xmm4,%xmm0
423744: 66 0f de c5 pmaxub %xmm5,%xmm0

And without,

423788: 66 41 0f 65 c2 pcmpgtw %xmm10,%xmm0
42378d: 66 0f 6f d8 movdqa %xmm0,%xmm3
423791: 66 0f df de pandn %xmm6,%xmm3
423795: 66 0f db f0 pand %xmm0,%xmm6
423799: 66 0f 71 e6 0f psraw $0xf,%xmm6
42379e: 66 0f df f0 pandn %xmm0,%xmm6
4237a2: 66 0f eb f3 por %xmm3,%xmm6

So, in that context when we were talking about LLVM being able to pattern match accurately, I was using this particular case as an example of why that may not always work. Though I agree with the replies above of not documenting usage patterns, and that being a good indicator of when to include an operation as a part of the proposal.

[dtig]

As this has had some discussion already, leaving a poll here to reach consensus.

In favor of including integer min/max operations in the current proposal, please respond with 👍
Against including integer min/max operations in the current proposal, please respond with 👎