languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 11:03:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

[ VM / typed_data ] Fixed inconsistent results for -Float32x4.zero().clamp and -Float64x2.zero().clamp

Browse source 
The fuzzer found an issue where clamping negative zero would result in
conflicting answers depending on whether or not the clamp operation had
been optimized in JIT mode.

The MINPS and MAXPS instructions will always return the second operand
if the values being compared are -0 and 0, which is the opposite of what
the C implementation of clamp was doing.

Fixes https://github.com/dart-lang/sdk/issues/40426.

Change-Id: I3afb725bd0c8643758dbe753d863ba93c86ad747
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134093
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
Ben Konyi 2020-02-03 21:59:48 +00:00
parent a0ca3e720e
commit 30f25ba735
5 changed files with 111 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
runtime/lib/simd128.cc
View file

@ -189,14 +189,31 @@ DEFINE_NATIVE_ENTRY(Float32x4_clamp, 0, 3) {
GET_NON_NULL_NATIVE_ARGUMENT(Float32x4, hi, arguments->NativeArgAt(2));
// The order of the clamping must match the order of the optimized code:
// MAX(MIN(self, hi), lo).
float _x = self.x() < hi.x() ? self.x() : hi.x();
float _y = self.y() < hi.y() ? self.y() : hi.y();
float _z = self.z() < hi.z() ? self.z() : hi.z();
float _w = self.w() < hi.w() ? self.w() : hi.w();
_x = _x < lo.x() ? lo.x() : _x;
_y = _y < lo.y() ? lo.y() : _y;
_z = _z < lo.z() ? lo.z() : _z;
_w = _w < lo.w() ? lo.w() : _w;
float _x;
float _y;
float _z;
float _w;
// ARM semantics are different from X86/X64 at an instruction level. Ensure
// that we match the semantics of the architecture in the C version.
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
_x = self.x() < hi.x() ? self.x() : hi.x();
_y = self.y() < hi.y() ? self.y() : hi.y();
_z = self.z() < hi.z() ? self.z() : hi.z();
_w = self.w() < hi.w() ? self.w() : hi.w();
_x = lo.x() < _x ? _x : lo.x();
_y = lo.y() < _y ? _y : lo.y();
_z = lo.z() < _z ? _z : lo.z();
_w = lo.w() < _w ? _w : lo.w();
#else
_x = fminf(self.x(), hi.x());
_y = fminf(self.y(), hi.y());
_z = fminf(self.z(), hi.z());
_w = fminf(self.w(), hi.w());
_x = fmaxf(_x, lo.x());
_y = fmaxf(_y, lo.y());
_z = fmaxf(_z, lo.z());
_w = fmaxf(_w, lo.w());
#endif
return Float32x4::New(_x, _y, _z, _w);
}
@ -719,10 +736,22 @@ DEFINE_NATIVE_ENTRY(Float64x2_clamp, 0, 3) {
GET_NON_NULL_NATIVE_ARGUMENT(Float64x2, hi, arguments->NativeArgAt(2));
// The order of the clamping must match the order of the optimized code:
// MAX(MIN(self, hi), lo).
double _x = self.x() < hi.x() ? self.x() : hi.x();
double _y = self.y() < hi.y() ? self.y() : hi.y();
_x = _x < lo.x() ? lo.x() : _x;
_y = _y < lo.y() ? lo.y() : _y;
double _x;
double _y;
// ARM semantics are different from X86/X64 at an instruction level. Ensure
// that we match the semantics of the architecture in the C version.
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
_x = self.x() < hi.x() ? self.x() : hi.x();
_y = self.y() < hi.y() ? self.y() : hi.y();
_x = lo.x() < _x ? _x : lo.x();
_y = lo.y() < _y ? _y : lo.y();
#else
_x = fminf(self.x(), hi.x());
_y = fminf(self.y(), hi.y());
_x = fmaxf(_x, lo.x());
_y = fmaxf(_y, lo.y());
#endif
return Float64x2::New(_x, _y);
}

6
runtime/vm/simulator_arm.cc
View file

@ -3081,15 +3081,13 @@ void Simulator::DecodeSIMDDataProcessing(Instr* instr) {
(instr->Bits(20, 2) == 2) && (instr->Bits(23, 2) == 0)) {
// Format(instr, "vminqs 'qd, 'qn, 'qm");
for (int i = 0; i < 4; i++) {
s8d.data_[i].f =
s8n.data_[i].f <= s8m.data_[i].f ? s8n.data_[i].f : s8m.data_[i].f;
s8d.data_[i].f = fminf(s8n.data_[i].f, s8m.data_[i].f);
}
} else if ((instr->Bits(8, 4) == 15) && (instr->Bit(4) == 0) &&
(instr->Bits(20, 2) == 0) && (instr->Bits(23, 2) == 0)) {
// Format(instr, "vmaxqs 'qd, 'qn, 'qm");
for (int i = 0; i < 4; i++) {
s8d.data_[i].f =
s8n.data_[i].f >= s8m.data_[i].f ? s8n.data_[i].f : s8m.data_[i].f;
s8d.data_[i].f = fmaxf(s8n.data_[i].f, s8m.data_[i].f);
}
} else if ((instr->Bits(8, 4) == 7) && (instr->Bit(4) == 0) &&
(instr->Bits(20, 2) == 3) && (instr->Bits(23, 2) == 3) &&

8
runtime/vm/simulator_arm64.cc
View file

@ -2857,11 +2857,11 @@ void Simulator::DecodeSIMDThreeSame(Instr* instr) {
} else if ((U == 0) && (opcode == 0x1e)) {
if (instr->Bit(23) == 1) {
// Format(instr, "vmin'vsz 'vd, 'vn, 'vm");
const float m = (vn_flt > vm_flt) ? vm_flt : vn_flt;
const float m = fminf(vn_flt, vm_flt);
res = bit_cast<int32_t, float>(m);
} else {
// Format(instr, "vmax'vsz 'vd, 'vn, 'vm");
const float m = (vn_flt < vm_flt) ? vm_flt : vn_flt;
const float m = fmaxf(vn_flt, vm_flt);
res = bit_cast<int32_t, float>(m);
}
} else if ((U == 0) && (opcode == 0x1f)) {
@ -2931,11 +2931,11 @@ void Simulator::DecodeSIMDThreeSame(Instr* instr) {
} else if ((U == 0) && (opcode == 0x1e)) {
if (instr->Bit(23) == 1) {
// Format(instr, "vmin'vsz 'vd, 'vn, 'vm");
const double m = (vn_dbl > vm_dbl) ? vm_dbl : vn_dbl;
const double m = fmin(vn_dbl, vm_dbl);
res = bit_cast<int64_t, double>(m);
} else {
// Format(instr, "vmax'vsz 'vd, 'vn, 'vm");
const double m = (vn_dbl < vm_dbl) ? vm_dbl : vn_dbl;
const double m = fmax(vn_dbl, vm_dbl);
res = bit_cast<int64_t, double>(m);
}
} else {

32
tests/lib/typed_data/float32x4_clamp_test.dart
View file

@ -31,9 +31,41 @@ void testClamp() {
Expect.equals(a.w, 0.0);
}
Float32x4 negativeZeroClamp() {
final negZero = -Float32x4.zero();
return negZero.clamp(negZero, Float32x4.zero());
}
Float32x4 zeroClamp() {
final negOne = -Float32x4(1.0, 1.0, 1.0, 1.0);
return Float32x4.zero().clamp(negOne, -Float32x4.zero());
}
// Regression test for https://github.com/dart-lang/sdk/issues/40426.
void testNegativeZeroClamp(Float32x4 unopt) {
final res = negativeZeroClamp();
Expect.equals(res.x.compareTo(unopt.x), 0);
Expect.equals(res.y.compareTo(unopt.y), 0);
Expect.equals(res.z.compareTo(unopt.z), 0);
Expect.equals(res.w.compareTo(unopt.w), 0);
}
// Regression test for https://github.com/dart-lang/sdk/issues/40426.
void testZeroClamp(Float32x4 unopt) {
final res = zeroClamp();
Expect.equals(res.x.compareTo(unopt.x), 0);
Expect.equals(res.y.compareTo(unopt.y), 0);
Expect.equals(res.z.compareTo(unopt.z), 0);
Expect.equals(res.w.compareTo(unopt.w), 0);
}
main() {
final unoptNegZeroClamp = negativeZeroClamp();
final unoptZeroClamp = zeroClamp();
for (int i = 0; i < 2000; i++) {
testClampLowerGreaterThanUpper();
testClamp();
testNegativeZeroClamp(unoptNegZeroClamp);
testZeroClamp(unoptZeroClamp);
}
}

32
tests/lib_2/typed_data/float32x4_clamp_test.dart
View file

@ -31,9 +31,41 @@ void testClamp() {
Expect.equals(a.w, 0.0);
}
Float32x4 negativeZeroClamp() {
final negZero = -Float32x4.zero();
return negZero.clamp(negZero, Float32x4.zero());
}
Float32x4 zeroClamp() {
final negOne = -Float32x4(1.0, 1.0, 1.0, 1.0);
return Float32x4.zero().clamp(negOne, -Float32x4.zero());
}
// Regression test for https://github.com/dart-lang/sdk/issues/40426.
void testNegativeZeroClamp(Float32x4 unopt) {
final res = negativeZeroClamp();
Expect.equals(res.x.compareTo(unopt.x), 0);
Expect.equals(res.y.compareTo(unopt.y), 0);
Expect.equals(res.z.compareTo(unopt.z), 0);
Expect.equals(res.w.compareTo(unopt.w), 0);
}
// Regression test for https://github.com/dart-lang/sdk/issues/40426.
void testZeroClamp(Float32x4 unopt) {
final res = zeroClamp();
Expect.equals(res.x.compareTo(unopt.x), 0);
Expect.equals(res.y.compareTo(unopt.y), 0);
Expect.equals(res.z.compareTo(unopt.z), 0);
Expect.equals(res.w.compareTo(unopt.w), 0);
}
main() {
final unoptNegZeroClamp = negativeZeroClamp();
final unoptZeroClamp = zeroClamp();
for (int i = 0; i < 2000; i++) {
testClampLowerGreaterThanUpper();
testClamp();
testNegativeZeroClamp(unoptNegZeroClamp);
testZeroClamp(unoptZeroClamp);
}
}