Skip to content

Commit cd6cf73

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 Float32x4 and Float64x2 clamp with min and max
clamp is exactly a min followed by a max, so no backend needs an implementation of its own. The VM drops its intrinsic, and on dart2js, DDC and dart2wasm clamp stops unpacking the lanes into doubles, so on wasm it now lowers to f32x4.min and f32x4.max. That fixes clamp, which dropped NaNs and picked the wrong zero on a tie. Tests added for both. TEST=lib/typed_data/float32x4_clamp_test lib/typed_data/float64x2_clamp_test lib/typed_data/float32x4_test Fixes: #40426 Issue: #64170 Issue: #63962 Change-Id: Id8c92c5021dc28af5c34a2000dd696aefad329dd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/549060 Reviewed-by: Martin Kustermann <kustermann@google.com> Auto-Submit: Modestas Valauskas <valauskasmodestas@gmail.com> Reviewed-by: Slava Egorov <vegorov@google.com> Commit-Queue: Slava Egorov <vegorov@google.com>
1 parent f7cb0fa commit cd6cf73

15 files changed

Lines changed: 78 additions & 379 deletions

File tree

‎runtime/lib/simd128.cc‎

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -64,73 +64,6 @@ DEFINE_NATIVE_ENTRY(Float32x4_abs, 0, 1) {
6464
return Float32x4::New(_x, _y, _z, _w);
6565
}
6666

67-
#if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_ARM)
68-
static float vminf(float f1, float f2) {
69-
if (f1 == f2) {
70-
// take care of (-0.0) < 0.0, (they are equal according to minss)
71-
auto result = signbit(f1) ? f1 : f2;
72-
return result;
73-
}
74-
auto result = f1 > f2 ? f2 : f1;
75-
return result;
76-
}
77-
78-
static float vmaxf(float f1, float f2) {
79-
if (f1 == f2) {
80-
// take care of (-0.0) < 0.0, (they are equal according to minss)
81-
auto result = signbit(f1) ? f2 : f1;
82-
return result;
83-
}
84-
auto result = f1 < f2 ? f2 : f1;
85-
return result;
86-
}
87-
#endif
88-
89-
DEFINE_NATIVE_ENTRY(Float32x4_clamp, 0, 3) {
90-
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, self, arguments->NativeArgAt(0));
91-
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, lo, arguments->NativeArgAt(1));
92-
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, hi, arguments->NativeArgAt(2));
93-
// The order of the clamping must match the order of the optimized code:
94-
// MAX(MIN(self, hi), lo).
95-
float _x;
96-
float _y;
97-
float _z;
98-
float _w;
99-
// ARM semantics are different from X86/X64 at an instruction level. Ensure
100-
// that we match the semantics of the architecture in the C version.
101-
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
102-
_x = self.x() < hi.x() ? self.x() : hi.x();
103-
_y = self.y() < hi.y() ? self.y() : hi.y();
104-
_z = self.z() < hi.z() ? self.z() : hi.z();
105-
_w = self.w() < hi.w() ? self.w() : hi.w();
106-
_x = lo.x() < _x ? _x : lo.x();
107-
_y = lo.y() < _y ? _y : lo.y();
108-
_z = lo.z() < _z ? _z : lo.z();
109-
_w = lo.w() < _w ? _w : lo.w();
110-
#elif defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_ARM)
111-
_x = vminf(self.x(), hi.x());
112-
_y = vminf(self.y(), hi.y());
113-
_z = vminf(self.z(), hi.z());
114-
_w = vminf(self.w(), hi.w());
115-
_x = vmaxf(_x, lo.x());
116-
_y = vmaxf(_y, lo.y());
117-
_z = vmaxf(_z, lo.z());
118-
_w = vmaxf(_w, lo.w());
119-
#elif defined(TARGET_ARCH_RISCV32) || defined(TARGET_ARCH_RISCV64)
120-
_x = fminf(self.x(), hi.x());
121-
_y = fminf(self.y(), hi.y());
122-
_z = fminf(self.z(), hi.z());
123-
_w = fminf(self.w(), hi.w());
124-
_x = fmaxf(_x, lo.x());
125-
_y = fmaxf(_y, lo.y());
126-
_z = fmaxf(_z, lo.z());
127-
_w = fmaxf(_w, lo.w());
128-
#else
129-
#error Unknown architecture.
130-
#endif
131-
return Float32x4::New(_x, _y, _z, _w);
132-
}
133-
13467
DEFINE_NATIVE_ENTRY(Float32x4_getX, 0, 1) {
13568
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, self, arguments->NativeArgAt(0));
13669
double value = static_cast<double>(self.x());
@@ -346,56 +279,6 @@ DEFINE_NATIVE_ENTRY(Float64x2_abs, 0, 1) {
346279
return Float64x2::New(_x, _y);
347280
}
348281

349-
#if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_ARM)
350-
static double vmind(double f1, double f2) {
351-
if (f1 == f2) {
352-
// take care of (-0.0) < 0.0, (they are equal according to minss)
353-
return signbit(f1) ? f1 : f2;
354-
}
355-
return f1 > f2 ? f2 : f1;
356-
}
357-
358-
static double vmaxd(double f1, double f2) {
359-
if (f1 == f2) {
360-
// take care of (-0.0) < 0.0, (they are equal according to minss)
361-
return signbit(f1) ? f2 : f1;
362-
}
363-
return f1 < f2 ? f2 : f1;
364-
}
365-
#endif
366-
367-
DEFINE_NATIVE_ENTRY(Float64x2_clamp, 0, 3) {
368-
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, self, arguments->NativeArgAt(0));
369-
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, lo, arguments->NativeArgAt(1));
370-
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, hi, arguments->NativeArgAt(2));
371-
// The order of the clamping must match the order of the optimized code:
372-
// MAX(MIN(self, hi), lo).
373-
double _x;
374-
double _y;
375-
376-
// ARM semantics are different from X86/X64 at an instruction level. Ensure
377-
// that we match the semantics of the architecture in the C version.
378-
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
379-
_x = self.x() < hi.x() ? self.x() : hi.x();
380-
_y = self.y() < hi.y() ? self.y() : hi.y();
381-
_x = lo.x() < _x ? _x : lo.x();
382-
_y = lo.y() < _y ? _y : lo.y();
383-
#elif defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_ARM)
384-
_x = vmind(self.x(), hi.x());
385-
_y = vmind(self.y(), hi.y());
386-
_x = vmaxd(_x, lo.x());
387-
_y = vmaxd(_y, lo.y());
388-
#elif defined(TARGET_ARCH_RISCV32) || defined(TARGET_ARCH_RISCV64)
389-
_x = fmin(self.x(), hi.x());
390-
_y = fmin(self.y(), hi.y());
391-
_x = fmax(_x, lo.x());
392-
_y = fmax(_y, lo.y());
393-
#else
394-
#error Unknown architecture.
395-
#endif
396-
return Float64x2::New(_x, _y);
397-
}
398-
399282
DEFINE_NATIVE_ENTRY(Float64x2_getX, 0, 1) {
400283
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, self, arguments->NativeArgAt(0));
401284
return Double::New(self.x());

‎runtime/vm/bootstrap_natives.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ namespace dart {
173173
V(Float32x4_negate, 1) \
174174
V(Float32x4_scale, 2) \
175175
V(Float32x4_abs, 1) \
176-
V(Float32x4_clamp, 3) \
177176
V(Float32x4_getX, 1) \
178177
V(Float32x4_getY, 1) \
179178
V(Float32x4_getZ, 1) \
@@ -190,7 +189,6 @@ namespace dart {
190189
V(Float64x2_fromDoubles, 2) \
191190
V(Float64x2_fromFloat32x4, 1) \
192191
V(Float64x2_abs, 1) \
193-
V(Float64x2_clamp, 3) \
194192
V(Float64x2_getX, 1) \
195193
V(Float64x2_getY, 1) \
196194
V(Float64x2_getSignMask, 1) \

‎runtime/vm/compiler/backend/il.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11344,8 +11344,6 @@ class LoadThreadInstr : public TemplateDefinition<0, NoThrow, PureAndMovable> {
1134411344
M(1, _, Float32x4Abs, (Float32x4), Float32x4) \
1134511345
M(1, _, Float64x2Abs, (Float64x2), Float64x2) \
1134611346
M(1, _, Int32x4Not, (Int32x4), Int32x4) \
11347-
M(3, _, Float32x4Clamp, (Float32x4, Float32x4, Float32x4), Float32x4) \
11348-
M(3, _, Float64x2Clamp, (Float64x2, Float64x2, Float64x2), Float64x2) \
1134911347
M(1, _, Float64x2GetX, (Float64x2), Double) \
1135011348
M(1, _, Float64x2GetY, (Float64x2), Double) \
1135111349
M(2, _, Float64x2WithX, (Float64x2, Double), Float64x2) \

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

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4910,43 +4910,6 @@ DEFINE_EMIT(Simd32x4ToSimd32x4Conversion, (SameAsFirstInput, QRegister left)) {
49104910
// code generation.
49114911
}
49124912

4913-
DEFINE_EMIT(
4914-
Float32x4Clamp,
4915-
(QRegister result, QRegister left, QRegister lower, QRegister upper)) {
4916-
__ vminqs(result, left, upper);
4917-
__ vmaxqs(result, result, lower);
4918-
}
4919-
4920-
DEFINE_EMIT(Float64x2Clamp,
4921-
(QRegisterView result,
4922-
QRegisterView left,
4923-
QRegisterView lower,
4924-
QRegisterView upper)) {
4925-
compiler::Label done0, done1;
4926-
// result = max(min(left, upper), lower) |
4927-
// lower if (upper is NaN || left is NaN) |
4928-
// upper if lower is NaN
4929-
__ vcmpd(left.d(0), upper.d(0));
4930-
__ vmstat();
4931-
__ vmovd(result.d(0), upper.d(0), GE);
4932-
__ vmovd(result.d(0), left.d(0), LT); // less than or unordered(NaN)
4933-
__ b(&done0, VS); // at least one argument was NaN
4934-
__ vcmpd(result.d(0), lower.d(0));
4935-
__ vmstat();
4936-
__ vmovd(result.d(0), lower.d(0), LE);
4937-
__ Bind(&done0);
4938-
4939-
__ vcmpd(left.d(1), upper.d(1));
4940-
__ vmstat();
4941-
__ vmovd(result.d(1), upper.d(1), GE);
4942-
__ vmovd(result.d(1), left.d(1), LT); // less than or unordered(NaN)
4943-
__ b(&done1, VS); // at least one argument was NaN
4944-
__ vcmpd(result.d(1), lower.d(1));
4945-
__ vmstat();
4946-
__ vmovd(result.d(1), lower.d(1), LE);
4947-
__ Bind(&done1);
4948-
}
4949-
49504913
// Low (< 7) Q registers are needed for the vmovs instruction.
49514914
// TODO(dartbug.com/30953) support register range constraints in the regalloc.
49524915
DEFINE_EMIT(Float32x4With,
@@ -5355,8 +5318,6 @@ DEFINE_EMIT(Int32x4Shift,
53555318
CASE(Float32x4ToInt32x4) \
53565319
CASE(Int32x4ToFloat32x4) \
53575320
____(Simd32x4ToSimd32x4Conversion) \
5358-
SIMPLE(Float32x4Clamp) \
5359-
SIMPLE(Float64x2Clamp) \
53605321
CASE(Float32x4WithX) \
53615322
CASE(Float32x4WithY) \
53625323
CASE(Float32x4WithZ) \

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4147,20 +4147,6 @@ DEFINE_EMIT(
41474147
__ vinss(r, 3, VTMP, 0);
41484148
}
41494149

4150-
DEFINE_EMIT(
4151-
Float32x4Clamp,
4152-
(VRegister result, VRegister value, VRegister lower, VRegister upper)) {
4153-
__ vmins(result, value, upper);
4154-
__ vmaxs(result, result, lower);
4155-
}
4156-
4157-
DEFINE_EMIT(
4158-
Float64x2Clamp,
4159-
(VRegister result, VRegister value, VRegister lower, VRegister upper)) {
4160-
__ vmind(result, value, upper);
4161-
__ vmaxd(result, result, lower);
4162-
}
4163-
41644150
DEFINE_EMIT(Float32x4With,
41654151
(VRegister result, VRegister replacement, VRegister value)) {
41664152
__ fcvtsd(VTMP, replacement);
@@ -4435,8 +4421,6 @@ DEFINE_EMIT(Int32x4Shift,
44354421
CASE(Float32x4Zero) \
44364422
CASE(Float64x2Zero) \
44374423
____(SimdZero) \
4438-
SIMPLE(Float32x4Clamp) \
4439-
SIMPLE(Float64x2Clamp) \
44404424
CASE(Float32x4WithX) \
44414425
CASE(Float32x4WithY) \
44424426
CASE(Float32x4WithZ) \

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4267,24 +4267,6 @@ DEFINE_EMIT(Float64x2Zero, (XmmRegister value)) {
42674267
__ xorpd(value, value);
42684268
}
42694269

4270-
DEFINE_EMIT(Float32x4Clamp,
4271-
(SameAsFirstInput,
4272-
XmmRegister left,
4273-
XmmRegister lower,
4274-
XmmRegister upper)) {
4275-
__ minps(left, upper);
4276-
__ maxps(left, lower);
4277-
}
4278-
4279-
DEFINE_EMIT(Float64x2Clamp,
4280-
(SameAsFirstInput,
4281-
XmmRegister left,
4282-
XmmRegister lower,
4283-
XmmRegister upper)) {
4284-
__ minpd(left, upper);
4285-
__ maxpd(left, lower);
4286-
}
4287-
42884270
DEFINE_EMIT(Int32x4FromInts,
42894271
(XmmRegister result, Register, Register, Register, Register)) {
42904272
// TODO(dartbug.com/30949) avoid transfer through memory.
@@ -4471,8 +4453,6 @@ DEFINE_EMIT(Int32x4Select,
44714453
SIMPLE(Int32x4FromBools) \
44724454
SIMPLE(Float32x4Zero) \
44734455
SIMPLE(Float64x2Zero) \
4474-
SIMPLE(Float32x4Clamp) \
4475-
SIMPLE(Float64x2Clamp) \
44764456
CASE(Int32x4GetX) \
44774457
CASE(Int32x4GetY) \
44784458
CASE(Int32x4GetZ) \

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4467,24 +4467,6 @@ DEFINE_EMIT(Float64x2Zero, (XmmRegister value)) {
44674467
__ ClearFpuRegister(value);
44684468
}
44694469

4470-
DEFINE_EMIT(Float32x4Clamp,
4471-
(SameAsFirstInput,
4472-
XmmRegister value,
4473-
XmmRegister lower,
4474-
XmmRegister upper)) {
4475-
__ minps(value, upper);
4476-
__ maxps(value, lower);
4477-
}
4478-
4479-
DEFINE_EMIT(Float64x2Clamp,
4480-
(SameAsFirstInput,
4481-
XmmRegister value,
4482-
XmmRegister lower,
4483-
XmmRegister upper)) {
4484-
__ minpd(value, upper);
4485-
__ maxpd(value, lower);
4486-
}
4487-
44884470
DEFINE_EMIT(Int32x4FromInts,
44894471
(XmmRegister result, Register, Register, Register, Register)) {
44904472
// TODO(dartbug.com/30949) avoid transfer through memory.
@@ -4742,8 +4724,6 @@ DEFINE_EMIT(Int32x4Shift,
47424724
SIMPLE(Int32x4FromBools) \
47434725
SIMPLE(Float32x4Zero) \
47444726
SIMPLE(Float64x2Zero) \
4745-
SIMPLE(Float32x4Clamp) \
4746-
SIMPLE(Float64x2Clamp) \
47474727
CASE(Int32x4GetX) \
47484728
CASE(Int32x4GetY) \
47494729
CASE(Int32x4GetZ) \

‎runtime/vm/compiler/call_specializer.cc‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2578,9 +2578,6 @@ class SimdLowering : public ValueObject {
25782578
case MethodRecognizer::kFloat32x4Max:
25792579
Float32x4Binary(Token::kMAX);
25802580
return true;
2581-
case MethodRecognizer::kFloat32x4Clamp:
2582-
DoubleClamp(kUnboxedFloat, 4);
2583-
return true;
25842581
case MethodRecognizer::kFloat32x4Scale:
25852582
UnboxVector(0, kUnboxedFloat, kDoubleCid, 4);
25862583
UnboxScalar(1, kUnboxedFloat, 4);
@@ -2698,9 +2695,6 @@ class SimdLowering : public ValueObject {
26982695
case MethodRecognizer::kFloat64x2Max:
26992696
Float64x2Binary(Token::kMAX);
27002697
return true;
2701-
case MethodRecognizer::kFloat64x2Clamp:
2702-
DoubleClamp(kUnboxedDouble, 2);
2703-
return true;
27042698
case MethodRecognizer::kFloat64x2GetSignMask:
27052699
// TODO(riscv)
27062700
return false;
@@ -2915,25 +2909,6 @@ class SimdLowering : public ValueObject {
29152909
}
29162910
}
29172911

2918-
void DoubleClamp(Representation rep, intptr_t n) {
2919-
UnboxVector(0, rep, kDoubleCid, n);
2920-
UnboxVector(1, rep, kDoubleCid, n);
2921-
UnboxVector(2, rep, kDoubleCid, n);
2922-
2923-
for (intptr_t lane = 0; lane < n; lane++) {
2924-
auto* mid = AddDefinition(new (zone()) BinaryDoubleOpInstr(
2925-
Token::kMIN, new (zone()) Value(in_[0][lane]),
2926-
new (zone()) Value(in_[2][lane]), call_->deopt_id(), call_->source(),
2927-
rep));
2928-
op_[lane] = AddDefinition(new (zone()) BinaryDoubleOpInstr(
2929-
Token::kMAX, new (zone()) Value(mid),
2930-
new (zone()) Value(in_[1][lane]), call_->deopt_id(), call_->source(),
2931-
rep));
2932-
}
2933-
2934-
BoxVector(rep, n);
2935-
}
2936-
29372912
void CompareAsMask(Representation rep, Token::Kind op, intptr_t n) {
29382913
for (intptr_t lane = 0; lane < n; lane++) {
29392914
op_[lane] = AddDefinition(new (zone()) CompareAsMaskInstr(
@@ -3469,7 +3444,6 @@ bool CallSpecializer::TryInlineRecognizedMethod(
34693444
receiver, graph_entry, entry, last, result);
34703445

34713446
case MethodRecognizer::kFloat32x4Abs:
3472-
case MethodRecognizer::kFloat32x4Clamp:
34733447
case MethodRecognizer::kFloat32x4FromDoubles:
34743448
case MethodRecognizer::kFloat32x4Equal:
34753449
case MethodRecognizer::kFloat32x4GetSignMask:
@@ -3498,7 +3472,6 @@ bool CallSpecializer::TryInlineRecognizedMethod(
34983472
case MethodRecognizer::kFloat32x4WithZ:
34993473
case MethodRecognizer::kFloat32x4Zero:
35003474
case MethodRecognizer::kFloat64x2Abs:
3501-
case MethodRecognizer::kFloat64x2Clamp:
35023475
case MethodRecognizer::kFloat64x2FromDoubles:
35033476
case MethodRecognizer::kFloat64x2GetSignMask:
35043477
case MethodRecognizer::kFloat64x2GetX:

‎runtime/vm/compiler/recognized_methods_list.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,6 @@ namespace dart {
493493
V(TypedDataLibrary, _Float32x4, reciprocal, Float32x4Reciprocal, 0xd4350ab2) \
494494
V(TypedDataLibrary, _Float32x4, unary-, Float32x4Negate, 0xe68eac52) \
495495
V(TypedDataLibrary, _Float32x4, abs, Float32x4Abs, 0xeb296688) \
496-
V(TypedDataLibrary, _Float32x4, clamp, Float32x4Clamp, 0x77b05a1d) \
497496
V(TypedDataLibrary, _Float32x4, withX, Float32x4WithX, 0x9b766656) \
498497
V(TypedDataLibrary, _Float32x4, withY, Float32x4WithY, 0xb889207c) \
499498
V(TypedDataLibrary, _Float32x4, withZ, Float32x4WithZ, 0xc7e2bf2f) \
@@ -507,7 +506,6 @@ namespace dart {
507506
V(TypedDataLibrary, _Float64x2, get:y, Float64x2GetY, 0x27adc893) \
508507
V(TypedDataLibrary, _Float64x2, unary-, Float64x2Negate, 0x3a5dc21a) \
509508
V(TypedDataLibrary, _Float64x2, abs, Float64x2Abs, 0x9a07af9e) \
510-
V(TypedDataLibrary, _Float64x2, clamp, Float64x2Clamp, 0xfdbefd73) \
511509
V(TypedDataLibrary, _Float64x2, sqrt, Float64x2Sqrt, 0x6ac8ad92) \
512510
V(TypedDataLibrary, _Float64x2, get:signMask, Float64x2GetSignMask, \
513511
0x7c4dfa2a) \

0 commit comments

Comments
 (0)