Skip to content

Commit 41464f0

Browse files
modulovaluedart-scoped@luci-project-accounts.iam.gserviceaccount.com
modulovalue
authored and
dart-scoped@luci-project-accounts.iam.gserviceaccount.com
committed
[typed_data] Implement IEEE min/max for Float32x4/Float64x2 and add an exhaustive test
Make Float32x4/Float64x2 min/max IEEE (propagate NaN, break the signed-zero tie as -0.0 < +0.0, matching dart:math and the WASM ops) across dart2wasm, dart2js/ddc, the VM software fallback, and the x64/ia32/riscv/arm32 optimized codegen. TEST=lib/typed_data/min_max_edge_cases_test Fixes: #63962 Fixes: #31048 Issue: #64170 Change-Id: I76aea80ef950b18415dadba7cfba7305e5055bba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/543520 Reviewed-by: Stephen Adams <sra@google.com> Auto-Submit: Modestas Valauskas <valauskasmodestas@gmail.com> Reviewed-by: Martin Kustermann <kustermann@google.com> Commit-Queue: Martin Kustermann <kustermann@google.com>
1 parent 99fcaff commit 41464f0

10 files changed

Lines changed: 815 additions & 95 deletions

File tree

‎runtime/lib/simd128.cc‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,37 @@
1111

1212
namespace dart {
1313

14+
static float IeeeMinF(float a, float b) {
15+
if (isnan(a)) return a;
16+
if (isnan(b)) return b;
17+
float ab = bit_cast<float>(bit_cast<uint32_t>(a) | bit_cast<uint32_t>(b));
18+
if (ab == 0.0f) return ab;
19+
return a < b ? a : b;
20+
}
21+
static float IeeeMaxF(float a, float b) {
22+
if (isnan(a)) return a;
23+
if (isnan(b)) return b;
24+
if (a == 0.0f && b == 0.0f) {
25+
return bit_cast<float>(bit_cast<uint32_t>(a) & bit_cast<uint32_t>(b));
26+
}
27+
return a > b ? a : b;
28+
}
29+
static double IeeeMinD(double a, double b) {
30+
if (isnan(a)) return a;
31+
if (isnan(b)) return b;
32+
double ab = bit_cast<double>(bit_cast<uint64_t>(a) | bit_cast<uint64_t>(b));
33+
if (ab == 0.0) return ab;
34+
return a < b ? a : b;
35+
}
36+
static double IeeeMaxD(double a, double b) {
37+
if (isnan(a)) return a;
38+
if (isnan(b)) return b;
39+
if (a == 0.0 && b == 0.0) {
40+
return bit_cast<double>(bit_cast<uint64_t>(a) & bit_cast<uint64_t>(b));
41+
}
42+
return a > b ? a : b;
43+
}
44+
1445
static void ThrowMaskRangeException(int64_t m) {
1546
if ((m < 0) || (m > 255)) {
1647
Exceptions::ThrowRangeError("mask", Integer::Handle(Integer::New(m)), 0,
@@ -353,20 +384,20 @@ DEFINE_NATIVE_ENTRY(Float32x4_setW, 0, 2) {
353384
DEFINE_NATIVE_ENTRY(Float32x4_min, 0, 2) {
354385
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, self, arguments->NativeArgAt(0));
355386
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, other, arguments->NativeArgAt(1));
356-
float _x = self.x() < other.x() ? self.x() : other.x();
357-
float _y = self.y() < other.y() ? self.y() : other.y();
358-
float _z = self.z() < other.z() ? self.z() : other.z();
359-
float _w = self.w() < other.w() ? self.w() : other.w();
387+
float _x = IeeeMinF(self.x(), other.x());
388+
float _y = IeeeMinF(self.y(), other.y());
389+
float _z = IeeeMinF(self.z(), other.z());
390+
float _w = IeeeMinF(self.w(), other.w());
360391
return Float32x4::New(_x, _y, _z, _w);
361392
}
362393

363394
DEFINE_NATIVE_ENTRY(Float32x4_max, 0, 2) {
364395
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, self, arguments->NativeArgAt(0));
365396
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, other, arguments->NativeArgAt(1));
366-
float _x = self.x() > other.x() ? self.x() : other.x();
367-
float _y = self.y() > other.y() ? self.y() : other.y();
368-
float _z = self.z() > other.z() ? self.z() : other.z();
369-
float _w = self.w() > other.w() ? self.w() : other.w();
397+
float _x = IeeeMaxF(self.x(), other.x());
398+
float _y = IeeeMaxF(self.y(), other.y());
399+
float _z = IeeeMaxF(self.z(), other.z());
400+
float _w = IeeeMaxF(self.w(), other.w());
370401
return Float32x4::New(_x, _y, _z, _w);
371402
}
372403

@@ -636,16 +667,16 @@ DEFINE_NATIVE_ENTRY(Float64x2_setY, 0, 2) {
636667
DEFINE_NATIVE_ENTRY(Float64x2_min, 0, 2) {
637668
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, self, arguments->NativeArgAt(0));
638669
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, other, arguments->NativeArgAt(1));
639-
double _x = self.x() < other.x() ? self.x() : other.x();
640-
double _y = self.y() < other.y() ? self.y() : other.y();
670+
double _x = IeeeMinD(self.x(), other.x());
671+
double _y = IeeeMinD(self.y(), other.y());
641672
return Float64x2::New(_x, _y);
642673
}
643674

644675
DEFINE_NATIVE_ENTRY(Float64x2_max, 0, 2) {
645676
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, self, arguments->NativeArgAt(0));
646677
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, other, arguments->NativeArgAt(1));
647-
double _x = self.x() > other.x() ? self.x() : other.x();
648-
double _y = self.y() > other.y() ? self.y() : other.y();
678+
double _x = IeeeMaxD(self.x(), other.x());
679+
double _y = IeeeMaxD(self.y(), other.y());
649680
return Float64x2::New(_x, _y);
650681
}
651682

‎runtime/vm/compiler/backend/il_arm.cc‎

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5066,33 +5066,51 @@ DEFINE_EMIT(Float64x2Binary,
50665066
case SimdOpInstr::kFloat64x2WithY:
50675067
__ vmovd(left.d(1), right.d(0));
50685068
break;
5069-
case SimdOpInstr::kFloat64x2Min: {
5070-
// X lane.
5071-
__ vcmpd(left.d(0), right.d(0));
5072-
__ vmstat();
5073-
__ vmovd(left.d(0), right.d(0), GE);
5074-
// Y lane.
5075-
__ vcmpd(left.d(1), right.d(1));
5076-
__ vmstat();
5077-
__ vmovd(left.d(1), right.d(1), GE);
5078-
break;
5079-
}
5080-
case SimdOpInstr::kFloat64x2Max: {
5081-
// X lane.
5082-
__ vcmpd(left.d(0), right.d(0));
5083-
__ vmstat();
5084-
__ vmovd(left.d(0), right.d(0), LE);
5085-
// Y lane.
5086-
__ vcmpd(left.d(1), right.d(1));
5087-
__ vmstat();
5088-
__ vmovd(left.d(1), right.d(1), LE);
5089-
break;
5090-
}
50915069
default:
50925070
UNREACHABLE();
50935071
}
50945072
}
50955073

5074+
// ARMv7 NEON has no double-precision min/max, so each lane of a Float64x2
5075+
// compares and conditionally moves. A comparison keeps the first operand when
5076+
// the operands are unordered, so it drops NaNs and mishandles the -0.0/+0.0
5077+
// tie. il_x64.cc solves the same problem on x86 by running the comparison both
5078+
// ways and combining them with OR for min and (a | b) - (a ^ b) for max. The
5079+
// subtract flips a disagreeing -0.0 tie to +0.0 and keeps NaNs.
5080+
DEFINE_EMIT(Float64x2MinMax,
5081+
(SameAsFirstInput,
5082+
QRegisterView left,
5083+
QRegisterView right,
5084+
Temp<QRegister> temp)) {
5085+
const QRegisterView other(temp);
5086+
const bool is_min = instr->kind() == SimdOpInstr::kFloat64x2Min;
5087+
const Condition cond = is_min ? GE : LE;
5088+
// other = comparison-min/max(right, left), lane by lane.
5089+
__ vmovq(other, right);
5090+
__ vcmpd(other.d(0), left.d(0));
5091+
__ vmstat();
5092+
__ vmovd(other.d(0), left.d(0), cond);
5093+
__ vcmpd(other.d(1), left.d(1));
5094+
__ vmstat();
5095+
__ vmovd(other.d(1), left.d(1), cond);
5096+
// left = comparison-min/max(left, right), lane by lane.
5097+
__ vcmpd(left.d(0), right.d(0));
5098+
__ vmstat();
5099+
__ vmovd(left.d(0), right.d(0), cond);
5100+
__ vcmpd(left.d(1), right.d(1));
5101+
__ vmstat();
5102+
__ vmovd(left.d(1), right.d(1), cond);
5103+
if (is_min) {
5104+
__ vorrq(left, left, other);
5105+
} else {
5106+
const QRegisterView xored(QTMP);
5107+
__ veorq(xored, left, other);
5108+
__ vorrq(left, left, other);
5109+
__ vsubd(left.d(0), left.d(0), xored.d(0));
5110+
__ vsubd(left.d(1), left.d(1), xored.d(1));
5111+
}
5112+
}
5113+
50965114
DEFINE_EMIT(Int32x4FromInts,
50975115
(QRegisterView result,
50985116
Register v0,
@@ -5355,9 +5373,10 @@ DEFINE_EMIT(Int32x4Shift,
53555373
CASE(Float64x2Scale) \
53565374
CASE(Float64x2WithX) \
53575375
CASE(Float64x2WithY) \
5376+
____(Float64x2Binary) \
53585377
CASE(Float64x2Min) \
53595378
CASE(Float64x2Max) \
5360-
____(Float64x2Binary) \
5379+
____(Float64x2MinMax) \
53615380
SIMPLE(Int32x4FromInts) \
53625381
SIMPLE(Int32x4FromBools) \
53635382
CASE(Int32x4GetX) \

‎runtime/vm/compiler/backend/il_ia32.cc‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4008,8 +4008,6 @@ Condition DoubleTestOpInstr::EmitConditionCode(FlowGraphCompiler* compiler,
40084008
SIMD_OP_FLOAT_ARITH(V, Sub, sub) \
40094009
SIMD_OP_FLOAT_ARITH(V, Mul, mul) \
40104010
SIMD_OP_FLOAT_ARITH(V, Div, div) \
4011-
SIMD_OP_FLOAT_ARITH(V, Min, min) \
4012-
SIMD_OP_FLOAT_ARITH(V, Max, max) \
40134011
V(Int32x4Add, addpl) \
40144012
V(Int32x4Sub, subpl) \
40154013
V(Int32x4BitAnd, andps) \
@@ -4088,6 +4086,78 @@ DEFINE_EMIT(SimdBinaryOp,
40884086
}
40894087
}
40904088

4089+
// minps doesn't propagate a NaN or -0.0 from its first operand, so run it both
4090+
// ways and OR the results. OR sets the sign to -0.0 when either operand is
4091+
// -0.0, and a NaN OR anything stays a NaN.
4092+
DEFINE_EMIT(Float32x4Min,
4093+
(SameAsFirstInput,
4094+
XmmRegister left,
4095+
XmmRegister right,
4096+
Temp<XmmRegister> temp)) {
4097+
__ movaps(temp, right); // temp = right
4098+
__ minps(temp, left); // temp = min(right, left)
4099+
__ minps(left, right); // left = min(left, right)
4100+
__ orps(left, temp); // left = left | temp
4101+
// We do not force NaN canonicalization, so we stop here.
4102+
}
4103+
4104+
// maxps doesn't propagate a NaN or +0.0 from its first operand, so run it both
4105+
// ways (max(right, left) in temp, max(left, right) in left) and compute
4106+
// (temp | left) - (temp ^ left). +0.0 for the tie would be temp & left, but
4107+
// AND-ing two different NaNs can make an infinity. The subtract flips a
4108+
// disagreeing -0.0 tie to +0.0, leaves agreeing zeros and ordinary values
4109+
// unchanged, and keeps NaNs.
4110+
DEFINE_EMIT(Float32x4Max,
4111+
(SameAsFirstInput,
4112+
XmmRegister left,
4113+
XmmRegister right,
4114+
Temp<XmmRegister> temp)) {
4115+
__ movaps(temp, right); // temp = right
4116+
__ maxps(temp, left); // temp = max(right, left)
4117+
__ maxps(left, right); // left = max(left, right)
4118+
__ xorps(left, temp); // left = temp ^ left
4119+
__ orps(temp, left); // temp |= left, rebuilding temp | left
4120+
__ subps(temp, left); // temp -= left, giving (temp | left) - (temp ^ left)
4121+
__ movaps(left, temp);
4122+
// We do not force NaN canonicalization, so we stop here.
4123+
}
4124+
4125+
// minpd doesn't propagate a NaN or -0.0 from its first operand, so run it both
4126+
// ways and OR the results. OR sets the sign to -0.0 when either operand is
4127+
// -0.0, and a NaN OR anything stays a NaN.
4128+
DEFINE_EMIT(Float64x2Min,
4129+
(SameAsFirstInput,
4130+
XmmRegister left,
4131+
XmmRegister right,
4132+
Temp<XmmRegister> temp)) {
4133+
__ movaps(temp, right); // temp = right
4134+
__ minpd(temp, left); // temp = min(right, left)
4135+
__ minpd(left, right); // left = min(left, right)
4136+
__ orpd(left, temp); // left = left | temp
4137+
// We do not force NaN canonicalization, so we stop here.
4138+
}
4139+
4140+
// maxpd doesn't propagate a NaN or +0.0 from its first operand, so run it both
4141+
// ways (max(right, left) in temp, max(left, right) in left) and compute
4142+
// (temp | left) - (temp ^ left). +0.0 for the tie would be temp & left, but
4143+
// AND-ing two different NaNs can make an infinity. The subtract flips a
4144+
// disagreeing -0.0 tie to +0.0, leaves agreeing zeros and ordinary values
4145+
// unchanged, and keeps NaNs.
4146+
DEFINE_EMIT(Float64x2Max,
4147+
(SameAsFirstInput,
4148+
XmmRegister left,
4149+
XmmRegister right,
4150+
Temp<XmmRegister> temp)) {
4151+
__ movaps(temp, right); // temp = right
4152+
__ maxpd(temp, left); // temp = max(right, left)
4153+
__ maxpd(left, right); // left = max(left, right)
4154+
__ xorpd(left, temp); // left = temp ^ left
4155+
__ orpd(temp, left); // temp |= left, rebuilding temp | left
4156+
__ subpd(temp, left); // temp -= left, giving (temp | left) - (temp ^ left)
4157+
__ movaps(left, temp);
4158+
// We do not force NaN canonicalization, so we stop here.
4159+
}
4160+
40914161
#define SIMD_OP_SIMPLE_UNARY(V) \
40924162
SIMD_OP_FLOAT_ARITH(V, Sqrt, sqrt) \
40934163
SIMD_OP_FLOAT_ARITH(V, Negate, negate) \
@@ -4371,6 +4441,10 @@ DEFINE_EMIT(Int32x4Select,
43714441
CASE(Float32x4WithZ) \
43724442
CASE(Float32x4WithW) \
43734443
____(SimdBinaryOp) \
4444+
SIMPLE(Float32x4Min) \
4445+
SIMPLE(Float32x4Max) \
4446+
SIMPLE(Float64x2Min) \
4447+
SIMPLE(Float64x2Max) \
43744448
SIMD_OP_SIMPLE_UNARY(CASE) \
43754449
CASE(Float32x4GetX) \
43764450
CASE(Float32x4GetY) \

‎runtime/vm/compiler/backend/il_riscv.cc‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4263,11 +4263,35 @@ void BinaryDoubleOpInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
42634263
__ fdivd(result, left, right);
42644264
break;
42654265
case Token::kMIN:
4266-
__ fmind(result, left, right);
4267-
break;
4268-
case Token::kMAX:
4269-
__ fmaxd(result, left, right);
4266+
case Token::kMAX: {
4267+
const bool is_min = op_kind() == Token::kMIN;
4268+
if (__ Supports(RV_Zfa)) {
4269+
if (is_min) {
4270+
__ fminmd(result, left, right);
4271+
} else {
4272+
__ fmaxmd(result, left, right);
4273+
}
4274+
} else {
4275+
// fmind/fmaxd return the non-NaN operand, but Float64x2 min/max must
4276+
// propagate NaNs. Return a NaN operand directly, as MathMinMax does.
4277+
compiler::Label done;
4278+
__ feqd(TMP, left, left);
4279+
__ fmvd(result, left);
4280+
ASSERT(result != left);
4281+
__ beqz(TMP, &done, compiler::Assembler::kNearJump);
4282+
__ feqd(TMP, right, right);
4283+
__ fmvd(result, right);
4284+
ASSERT(result != right);
4285+
__ beqz(TMP, &done, compiler::Assembler::kNearJump);
4286+
if (is_min) {
4287+
__ fmind(result, left, right);
4288+
} else {
4289+
__ fmaxd(result, left, right);
4290+
}
4291+
__ Bind(&done);
4292+
}
42704293
break;
4294+
}
42714295
default:
42724296
UNREACHABLE();
42734297
}
@@ -4287,11 +4311,35 @@ void BinaryDoubleOpInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
42874311
__ fdivs(result, left, right);
42884312
break;
42894313
case Token::kMIN:
4290-
__ fmins(result, left, right);
4291-
break;
4292-
case Token::kMAX:
4293-
__ fmaxs(result, left, right);
4314+
case Token::kMAX: {
4315+
const bool is_min = op_kind() == Token::kMIN;
4316+
if (__ Supports(RV_Zfa)) {
4317+
if (is_min) {
4318+
__ fminms(result, left, right);
4319+
} else {
4320+
__ fmaxms(result, left, right);
4321+
}
4322+
} else {
4323+
// fmins/fmaxs return the non-NaN operand, but Float32x4 min/max must
4324+
// propagate NaNs. Return a NaN operand directly, as MathMinMax does.
4325+
compiler::Label done;
4326+
__ feqs(TMP, left, left);
4327+
__ fmvs(result, left);
4328+
ASSERT(result != left);
4329+
__ beqz(TMP, &done, compiler::Assembler::kNearJump);
4330+
__ feqs(TMP, right, right);
4331+
__ fmvs(result, right);
4332+
ASSERT(result != right);
4333+
__ beqz(TMP, &done, compiler::Assembler::kNearJump);
4334+
if (is_min) {
4335+
__ fmins(result, left, right);
4336+
} else {
4337+
__ fmaxs(result, left, right);
4338+
}
4339+
__ Bind(&done);
4340+
}
42944341
break;
4342+
}
42954343
default:
42964344
UNREACHABLE();
42974345
}

0 commit comments

Comments
 (0)