simd icon indicating copy to clipboard operation
simd copied to clipboard

Inefficient x64 codegen for fmin/fmax

Open abrown opened this issue 5 years ago • 11 comments

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?

abrown avatar Feb 06 '20 19:02 abrown

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.

ngzhian avatar Feb 06 '20 20:02 ngzhian

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?

abrown avatar Feb 06 '20 21:02 abrown

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

penzn avatar Feb 06 '20 22:02 penzn

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.

dtig avatar Feb 18 '20 19:02 dtig

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

abrown avatar Feb 21 '20 19:02 abrown

@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?

abrown avatar Feb 21 '20 21:02 abrown

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.

lars-t-hansen avatar Feb 03 '21 10:02 lars-t-hansen

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.

penzn avatar Oct 23 '21 00:10 penzn

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

Maratyszcza avatar Oct 23 '21 05:10 Maratyszcza

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

penzn avatar Oct 26 '21 00:10 penzn

(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:

  • Signed zero handling (ex.: min(0.0, -0.0) == -0.0)
  • NaN propagation (if either input is NaN output is NaN)
  • NaN canonicalization

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

  • cmpunordps to detect NaN inputs
  • and or or to handle signed zeros
  • Performing operation twice to detect asymmetry
  • Various forms of blending to put the result back together

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:

  • vrange has the desired behavior v.r.t signed zeros
  • vfixupimm detects and corrects special values, but requires extra movs and registers to set up fixup table
  • Masked operations

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

  • Both get close to theoretical maxiumum performance
  • vrange combined with conditional ops wins

Implementation challenges:

  • AVX512 detection and assembler changes
    • cpuid, encoding, tests, etc
  • For conditional operations, mask register representation
    • Possible to hardcode if this is the only use

penzn avatar Dec 07 '21 03:12 penzn