WebAssembly / simd

Branch of the spec repo scoped to discussion of SIMD in WebAssembly
Other
530 stars 43 forks source link

Inefficient x64 codegen for fmin/fmax #186

Open abrown opened 4 years ago

abrown commented 4 years ago

In attempting to implement fmin and fmax, I observed that the semantics of these instructions prevents a single instruction lowering on x64. V8 has a 9-instruction lowering for F32x4Min, for example, and the other min/max implementations for F32x4/F64x2 are not better.

Also, I noticed that the V8 implementation quiets and clears the NaN payload; this behavior does not seem to be specified in the spec but I suspect that it is necessary for passing the spec tests. Is this correct?

ngzhian commented 4 years ago

I think the key point from the proposal is "They are lane-wise versions of the existing scalar WebAssembly operations." This requires it to follow the semantics of scalar fmin.

abrown commented 4 years ago

I think the key point from the proposal is "They are lane-wise versions of the existing scalar WebAssembly operations."

I guess we could discuss if that has to be the case. Is that the reason behind quieting and clearing the NaN payload?

penzn commented 4 years ago

Yes, wasm propagates NaNs, that's why they have to be of "quiet" type.

dtig commented 4 years ago

This has been discussed a few times in various contexts, and there isn't much that can be done here that's actionable. It may be possible to whittle down the V8 codegen some more (if you know of something specific here, please file a V8 issue so we can address it), but the conclusion of most previous discussions here has been that we need fmin/fmax operations, and at least for the MVP SIMD, these have to be deterministic, and follow the same semantics of the scalar operations.

See also the discussion for Pseudo Min/Max operations in #122, as a potential follow up to this particular issue. I'm closing this for now as your question seems to be answered, feel free to reopen if there's still something outstanding to address that hasn't already been addressed here or in #122.

abrown commented 4 years ago

@penzn, could VFIXUPIMM* be used here to do this quieting/clearing?

abrown commented 4 years ago

@dtig, could we keep this open or tag this in some way so that we keep track of these inefficiencies? @rrwinterton had proposed an optimization guide for documenting pitfalls exactly like these--alternately we could just start recording issues like this one there?

lars-t-hansen commented 3 years ago

I agree with @abrown, we should reopen this since it fits in with the set of about 10 other issues we have on related topics.

I don't know if fmin/fmax with one constant operand is typical in code, but it does lend itself to better code and may be worth specializing.

penzn commented 2 years ago

After looking into this a little more, I think there is a bigger problem than NaNs, specifically comparisons of -0.0 to +0.0. There are known ways to deal with (consistently ignore or propagate) NaN values, but less so for 'sign of zero' situations. From what I can tell, this is taken from JS spec, while native standards often define sign of zero coming from min/max as implementation-dependent. I am not sure what is the practical use in enforcing sign of zero, since from equality point of view it is the same value.

Maratyszcza commented 2 years ago

VRANGEPS/VRANGEPD from AVX512 might be helpful, as their handling of signed zeroes is consistent with WAsm instructions.

penzn commented 2 years ago

Yeah, that is on my list as well - trying to investigate better min/max sequences, using AVX512.

penzn commented 2 years ago

(the following is an excerpt from meeting notes https://github.com/WebAssembly/relaxed-simd/issues/47)

Floating-point min and max operations in WebAssembly have the following constraints:

This makes x86-based implementations somewhat complicated, because SSE and AVX instructions don't honor these properties. Solutions usually include:

The problem is that mitigations for the constraints above interact with each other, making overall sequence more complicated. Removing even one of the constraints will make the sequence much simpler. Using more registers may help as well.

With AVX512:

We have tried vrange and vrange with conditional lane operations, on a MobileNet demo:

Implementation challenges: