Skip to content

Commit

Permalink
Bug 1669964 - Fix code generation for pmin/pmax on x86. r=jseward
Browse files Browse the repository at this point in the history
pmin and pmax handled NaN incorrectly because the wonky semantics of
MINPS and MAXPS require arguments to be reversed to properly handle
NaN for these wasm SIMD instructions.

We fix this the expedient way: we keep the same masm abstraction and
introduce moves to rearrange the arguments.  This should be optimized
eventually and a bug will be filed for that now.

Differential Revision: https://phabricator.services.mozilla.com/D92927
  • Loading branch information
Lars T Hansen committed Oct 14, 2020
1 parent dec2b41 commit 49c0d8a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
4 changes: 2 additions & 2 deletions js/src/jit-test/tests/wasm/simd/experimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ function V128StoreExpr(addr, v) {
}

// Pseudo-min/max, https://github.com/WebAssembly/simd/pull/122
var fxs = [5, 1, -4, 2];
var fxs = [5, 1, -4, NaN];
var fys = [6, 0, -7, 3];
var dxs = [5, 1];
var dxs = [5, NaN];
var dys = [6, 0];

for ( let [opcode, xs, ys, operator] of [[F32x4PMinCode, fxs, fys, pmin],
Expand Down
20 changes: 16 additions & 4 deletions js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1981,24 +1981,36 @@ void MacroAssembler::maxFloat64x2(FloatRegister rhs, FloatRegister lhsDest,

void MacroAssembler::pseudoMinFloat32x4(FloatRegister rhs,
FloatRegister lhsDest) {
vminps(Operand(rhs), lhsDest, lhsDest);
ScratchSimd128Scope scratch(*this);
vmovaps(rhs, scratch);
vminps(Operand(lhsDest), scratch, scratch);
vmovaps(scratch, lhsDest);
}

void MacroAssembler::pseudoMinFloat64x2(FloatRegister rhs,
FloatRegister lhsDest) {
vminpd(Operand(rhs), lhsDest, lhsDest);
ScratchSimd128Scope scratch(*this);
vmovapd(rhs, scratch);
vminpd(Operand(lhsDest), scratch, scratch);
vmovapd(scratch, lhsDest);
}

// Compare-based maximum

void MacroAssembler::pseudoMaxFloat32x4(FloatRegister rhs,
FloatRegister lhsDest) {
vmaxps(Operand(rhs), lhsDest, lhsDest);
ScratchSimd128Scope scratch(*this);
vmovaps(rhs, scratch);
vmaxps(Operand(lhsDest), scratch, scratch);
vmovaps(scratch, lhsDest);
}

void MacroAssembler::pseudoMaxFloat64x2(FloatRegister rhs,
FloatRegister lhsDest) {
vmaxpd(Operand(rhs), lhsDest, lhsDest);
ScratchSimd128Scope scratch(*this);
vmovapd(rhs, scratch);
vmaxpd(Operand(lhsDest), scratch, scratch);
vmovapd(scratch, lhsDest);
}

void MacroAssembler::widenDotInt16x8(FloatRegister rhs, FloatRegister lhsDest) {
Expand Down

0 comments on commit 49c0d8a

Please sign in to comment.