From b370e3f5096c2d6c24410ff9b48f6b2340fb90e6 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Tue, 3 Oct 2023 15:32:30 +0000 Subject: [PATCH] [vm, compiler] Fix Float32x4.greaterThan[OrEqual] on IA32 and X64 to give the expected result for NaNs. TEST=lib/typed_data/float32x4_compare_test Bug: https://github.com/dart-lang/sdk/issues/53662 Change-Id: Ia5e1e5088fde84c60d30e0a1d50d1d2d3b50f2f0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328768 Reviewed-by: Alexander Markov Commit-Queue: Ryan Macnak --- runtime/vm/compiler/backend/il.cc | 19 +++ runtime/vm/compiler/backend/il_ia32.cc | 6 +- runtime/vm/compiler/backend/il_x64.cc | 6 +- .../typed_data/float32x4_compare_test.dart | 116 ++++++++++++++++++ 4 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 tests/lib/typed_data/float32x4_compare_test.dart diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index f4d3f94400b..2fdab81d4de 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -7920,6 +7920,24 @@ SimdOpInstr* SimdOpInstr::CreateFromCall(Zone* zone, case MethodRecognizer::kFloat64x2Sub: op = new (zone) SimdOpInstr(KindForOperator(kind), call->deopt_id()); break; +#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64) + case MethodRecognizer::kFloat32x4GreaterThan: + // cmppsgt does not exist, cmppsnlt gives wrong NaN result, need to flip + // at the IL level to get the right SameAsFirstInput. + op = new (zone) + SimdOpInstr(SimdOpInstr::kFloat32x4LessThan, call->deopt_id()); + op->SetInputAt(0, call->ArgumentValueAt(1)->CopyWithType(zone)); + op->SetInputAt(1, new (zone) Value(receiver)); + return op; + case MethodRecognizer::kFloat32x4GreaterThanOrEqual: + // cmppsge does not exist, cmppsnle gives wrong NaN result, need to flip + // at the IL level to get the right SameAsFirstInput. + op = new (zone) + SimdOpInstr(SimdOpInstr::kFloat32x4LessThanOrEqual, call->deopt_id()); + op->SetInputAt(0, call->ArgumentValueAt(1)->CopyWithType(zone)); + op->SetInputAt(1, new (zone) Value(receiver)); + return op; +#endif default: op = new (zone) SimdOpInstr(KindForMethod(kind), call->deopt_id()); break; @@ -7935,6 +7953,7 @@ SimdOpInstr* SimdOpInstr::CreateFromCall(Zone* zone, op->set_mask(mask); } ASSERT(call->ArgumentCount() == (op->InputCount() + (op->HasMask() ? 1 : 0))); + return op; } diff --git a/runtime/vm/compiler/backend/il_ia32.cc b/runtime/vm/compiler/backend/il_ia32.cc index f4d8e082cd3..7b80097c7b9 100644 --- a/runtime/vm/compiler/backend/il_ia32.cc +++ b/runtime/vm/compiler/backend/il_ia32.cc @@ -4052,8 +4052,6 @@ Condition DoubleTestOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler, V(Int32x4BitXor, xorps) \ V(Float32x4Equal, cmppseq) \ V(Float32x4NotEqual, cmppsneq) \ - V(Float32x4GreaterThan, cmppsnle) \ - V(Float32x4GreaterThanOrEqual, cmppsnlt) \ V(Float32x4LessThan, cmppslt) \ V(Float32x4LessThanOrEqual, cmppsle) @@ -4423,6 +4421,8 @@ LocationSummary* SimdOpInstr::MakeLocationSummary(Zone* zone, bool opt) const { #undef CASE #undef EMIT #undef SIMPLE + case SimdOpInstr::kFloat32x4GreaterThan: + case SimdOpInstr::kFloat32x4GreaterThanOrEqual: case kIllegalSimdOp: UNREACHABLE(); break; @@ -4442,6 +4442,8 @@ void SimdOpInstr::EmitNativeCode(FlowGraphCompiler* compiler) { #undef CASE #undef EMIT #undef SIMPLE + case SimdOpInstr::kFloat32x4GreaterThan: + case SimdOpInstr::kFloat32x4GreaterThanOrEqual: case kIllegalSimdOp: UNREACHABLE(); break; diff --git a/runtime/vm/compiler/backend/il_x64.cc b/runtime/vm/compiler/backend/il_x64.cc index d05c423ccf3..0ab9e8837f4 100644 --- a/runtime/vm/compiler/backend/il_x64.cc +++ b/runtime/vm/compiler/backend/il_x64.cc @@ -4228,8 +4228,6 @@ Condition DoubleTestOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler, V(Int32x4BitXor, xorps) \ V(Float32x4Equal, cmppseq) \ V(Float32x4NotEqual, cmppsneq) \ - V(Float32x4GreaterThan, cmppsnle) \ - V(Float32x4GreaterThanOrEqual, cmppsnlt) \ V(Float32x4LessThan, cmppslt) \ V(Float32x4LessThanOrEqual, cmppsle) @@ -4600,6 +4598,8 @@ LocationSummary* SimdOpInstr::MakeLocationSummary(Zone* zone, bool opt) const { #undef CASE #undef EMIT #undef SIMPLE + case SimdOpInstr::kFloat32x4GreaterThan: + case SimdOpInstr::kFloat32x4GreaterThanOrEqual: case kIllegalSimdOp: break; } @@ -4618,6 +4618,8 @@ void SimdOpInstr::EmitNativeCode(FlowGraphCompiler* compiler) { #undef CASE #undef EMIT #undef SIMPLE + case SimdOpInstr::kFloat32x4GreaterThan: + case SimdOpInstr::kFloat32x4GreaterThanOrEqual: case kIllegalSimdOp: UNREACHABLE(); break; diff --git a/tests/lib/typed_data/float32x4_compare_test.dart b/tests/lib/typed_data/float32x4_compare_test.dart new file mode 100644 index 00000000000..c7898f0c9b0 --- /dev/null +++ b/tests/lib/typed_data/float32x4_compare_test.dart @@ -0,0 +1,116 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// VMOptions=--max_deoptimization_counter_threshold=1000 --optimization-counter-threshold=10 --no-background-compilation +// VMOptions=--no-intrinsify + +import "dart:typed_data"; +import "package:expect/expect.dart"; + +testEqual() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.equal(b); + var d = b.equal(a); + Expect.equals(0, c.x); + Expect.equals(0, c.y); + Expect.equals(0, c.z); + Expect.equals(0, c.w); + Expect.equals(0, d.x); + Expect.equals(0, d.y); + Expect.equals(0, d.z); + Expect.equals(0, d.w); +} + +testNotEqual() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.notEqual(b); + var d = b.notEqual(a); + Expect.equals(-1, c.x); + Expect.equals(-1, c.y); + Expect.equals(-1, c.z); + Expect.equals(-1, c.w); + Expect.equals(-1, d.x); + Expect.equals(-1, d.y); + Expect.equals(-1, d.z); + Expect.equals(-1, d.w); +} + +testLessThan() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.lessThan(b); + var d = b.lessThan(a); + Expect.equals(0, c.x); + Expect.equals(0, c.y); + Expect.equals(-1, c.z); + Expect.equals(0, c.w); + Expect.equals(0, d.x); + Expect.equals(-1, d.y); + Expect.equals(0, d.z); + Expect.equals(0, d.w); +} + +testLessThanOrEqual() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.lessThanOrEqual(b); + var d = b.lessThanOrEqual(a); + Expect.equals(0, c.x); + Expect.equals(0, c.y); + Expect.equals(-1, c.z); + Expect.equals(0, c.w); + Expect.equals(0, d.x); + Expect.equals(-1, d.y); + Expect.equals(0, d.z); + Expect.equals(0, d.w); +} + +testGreaterThan() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.greaterThan(b); + var d = b.greaterThan(a); + Expect.equals(0, c.x); + Expect.equals(-1, c.y); + Expect.equals(0, c.z); + Expect.equals(0, c.w); + Expect.equals(0, d.x); + Expect.equals(0, d.y); + Expect.equals(-1, d.z); + Expect.equals(0, d.w); +} + +testGreaterThanOrEqual() { + var a = new Float32x4( + double.nan, double.infinity, double.negativeInfinity, double.nan); + var b = new Float32x4(0.0, 0.0, 0.0, double.nan); + var c = a.greaterThanOrEqual(b); + var d = b.greaterThan(a); + Expect.equals(0, c.x); + Expect.equals(-1, c.y); + Expect.equals(0, c.z); + Expect.equals(0, c.w); + Expect.equals(0, d.x); + Expect.equals(0, d.y); + Expect.equals(-1, d.z); + Expect.equals(0, d.w); +} + +main() { + for (int i = 0; i < 20; i++) { + testEqual(); + testNotEqual(); + testLessThan(); + testLessThanOrEqual(); + testGreaterThan(); + testGreaterThanOrEqual(); + } +}