From 0250ea3e9fd244ee6703583860254c953640c530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Mon, 13 Aug 2018 08:21:03 -0700 Subject: [PATCH] Bug 1482359: Use more JSOP_STRICTEQ optimizations for Object.is(). r=jandem --- .../tests/ion/inlining/object-is-stricteq.js | 143 ++++++++++++++++++ js/src/jit/IonBuilder.cpp | 60 ++++++-- js/src/jit/IonBuilder.h | 2 +- js/src/jit/MCallOptimize.cpp | 42 ++--- 4 files changed, 211 insertions(+), 36 deletions(-) create mode 100644 js/src/jit-test/tests/ion/inlining/object-is-stricteq.js diff --git a/js/src/jit-test/tests/ion/inlining/object-is-stricteq.js b/js/src/jit-test/tests/ion/inlining/object-is-stricteq.js new file mode 100644 index 000000000000..059c3f210c0d --- /dev/null +++ b/js/src/jit-test/tests/ion/inlining/object-is-stricteq.js @@ -0,0 +1,143 @@ +// Test when Object.is() is inlined as JSOP_STRICTEQ + +function SameValue(x, y) { + if (x === y) { + return (x !== 0) || (1 / x === 1 / y); + } + return (x !== x && y !== y); +} + +var compareTemplate = function compare(name, xs, ys) { + // Compare each entry in xs with each entry in ys and ensure Object.is + // computes the same result as SameValue. + for (var i = 0; i < 1000; ++i) { + var xi = (i % xs.length) | 0; + var yi = ((i + ((i / ys.length) | 0)) % ys.length) | 0; + + assertEq(Object.is(xs[xi], ys[yi]), SameValue(xs[xi], ys[yi]), name); + } +} + +const objects = { + plain: {}, + function: function(){}, + proxy: new Proxy({}, {}), +}; + +const sym = Symbol(); + +const testCases = [ + { + name: "Homogenous-Int32", + xs: [-1, 0, 1, 2, 0x7fffffff], + ys: [2, 1, 0, -5, -0x80000000], + }, + { + name: "Homogenous-Boolean", + xs: [true, false], + ys: [true, false], + }, + { + name: "Homogenous-Object", + xs: [{}, [], objects.plain, objects.proxy], + ys: [{}, objects.function, objects.plain, objects.proxy], + }, + { + name: "Homogenous-String", + xs: ["", "abc", "αβγαβγ", "𝐀𝐁𝐂𝐀𝐁𝐂", "ABCabc"], + ys: ["abc", "ABC", "ABCABC", "αβγαβγ", "𝐀𝐁𝐂𝐀𝐁𝐂"], + }, + { + name: "Homogenous-Symbol", + xs: [Symbol.iterator, Symbol(), Symbol.for("object-is"), sym], + ys: [sym, Symbol.match, Symbol(), Symbol.for("object-is-two")], + }, + { + name: "Homogenous-Null", + xs: [null, null], + ys: [null, null], + }, + { + name: "Homogenous-Undefined", + xs: [undefined, undefined], + ys: [undefined, undefined], + }, + + // Note: Values must not include floating-point types! + { + name: "String-Value", + xs: ["", "abc", "αβγαβγ", "𝐀𝐁𝐂𝐀𝐁𝐂"], + ys: [null, undefined, sym, true, 0, "", {}], + }, + { + name: "Null-Value", + xs: [null, null], + ys: [null, undefined, sym, true, 0, "", {}], + }, + { + name: "Undefined-Value", + xs: [undefined, undefined], + ys: [null, undefined, sym, true, 0, "", {}], + }, + { + name: "Boolean-Value", + xs: [true, false], + ys: [null, undefined, sym, true, 0, "", {}], + }, + + // Note: Values must not include floating-point types! + { + name: "Value-String", + xs: [null, undefined, sym, true, 0, "", {}], + ys: ["", "abc", "αβγαβγ", "𝐀𝐁𝐂𝐀𝐁𝐂"], + }, + { + name: "Value-Null", + xs: [null, undefined, sym, true, 0, "", {}], + ys: [null, null], + }, + { + name: "Value-Undefined", + xs: [null, undefined, sym, true, 0, "", {}], + ys: [undefined, undefined], + }, + { + name: "Value-Boolean", + xs: [null, undefined, sym, true, 0, "", {}], + ys: [undefined, undefined], + }, + + // Strict-equal comparison can be optimized to bitwise comparison when + // string types are not possible. + // Note: Values must not include floating-point types! + { + name: "Value-Value", + xs: [null, undefined, sym, true, 0, {}], + ys: [null, undefined, sym, true, 0, {}], + }, + { + name: "ValueMaybeString-ValueMaybeString", + xs: [null, undefined, sym, true, 0, "", {}], + ys: [null, undefined, sym, true, 0, "", {}], + }, + { + name: "Value-ValueMaybeString", + xs: [null, undefined, sym, true, 0, {}], + ys: [null, undefined, sym, true, 0, "", {}], + }, + { + name: "ValueMaybeString-Value", + xs: [null, undefined, sym, true, 0, "", {}], + ys: [null, undefined, sym, true, 0, {}], + }, +]; + +for (let {name, xs, ys} of testCases) { + // Create a separate function for each test case. + // Use indirect eval to avoid possible direct eval deopts. + const compare = (0, eval)(`(${compareTemplate})`); + + for (let i = 0; i < 5; ++i) { + compare(name, xs, ys); + } +} diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index ed8fe224fc7a..3811fe9d8ecd 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -5815,11 +5815,17 @@ IonBuilder::jsop_compare(JSOp op) AbortReasonOr IonBuilder::jsop_compare(JSOp op, MDefinition* left, MDefinition* right) { + // TODO: Support tracking optimizations for inlining a call and regular + // optimization tracking at the same time. Currently just drop optimization + // tracking when that happens. + bool canTrackOptimization = !IsCallPC(pc); + bool emitted = false; - startTrackingOptimizations(); + if (canTrackOptimization) + startTrackingOptimizations(); if (!forceInlineCaches()) { - MOZ_TRY(compareTrySpecialized(&emitted, op, left, right, true)); + MOZ_TRY(compareTrySpecialized(&emitted, op, left, right)); if (emitted) return Ok(); MOZ_TRY(compareTryBitwise(&emitted, op, left, right)); @@ -5834,7 +5840,8 @@ IonBuilder::jsop_compare(JSOp op, MDefinition* left, MDefinition* right) if (emitted) return Ok(); - trackOptimizationAttempt(TrackedStrategy::Compare_Call); + if (canTrackOptimization) + trackOptimizationAttempt(TrackedStrategy::Compare_Call); // Not possible to optimize. Do a slow vm call. MCompare* ins = MCompare::New(alloc(), left, right, op); @@ -5845,7 +5852,8 @@ IonBuilder::jsop_compare(JSOp op, MDefinition* left, MDefinition* right) if (ins->isEffectful()) MOZ_TRY(resumeAfter(ins)); - trackOptimizationSuccess(); + if (canTrackOptimization) + trackOptimizationSuccess(); return Ok(); } @@ -5862,10 +5870,14 @@ ObjectOrSimplePrimitive(MDefinition* op) } AbortReasonOr -IonBuilder::compareTrySpecialized(bool* emitted, JSOp op, MDefinition* left, MDefinition* right, - bool canTrackOptimization) +IonBuilder::compareTrySpecialized(bool* emitted, JSOp op, MDefinition* left, MDefinition* right) { MOZ_ASSERT(*emitted == false); + + // TODO: Support tracking optimizations for inlining a call and regular + // optimization tracking at the same time. Currently just drop optimization + // tracking when that happens. + bool canTrackOptimization = !IsCallPC(pc); if (canTrackOptimization) trackOptimizationAttempt(TrackedStrategy::Compare_SpecializedTypes); @@ -5911,21 +5923,29 @@ AbortReasonOr IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefinition* right) { MOZ_ASSERT(*emitted == false); - trackOptimizationAttempt(TrackedStrategy::Compare_Bitwise); + + // TODO: Support tracking optimizations for inlining a call and regular + // optimization tracking at the same time. Currently just drop optimization + // tracking when that happens. + bool canTrackOptimization = !IsCallPC(pc); + if (canTrackOptimization) + trackOptimizationAttempt(TrackedStrategy::Compare_Bitwise); // Try to emit a bitwise compare. Check if a bitwise compare equals the wanted // result for all observed operand types. // Only allow loose and strict equality. if (op != JSOP_EQ && op != JSOP_NE && op != JSOP_STRICTEQ && op != JSOP_STRICTNE) { - trackOptimizationOutcome(TrackedOutcome::RelationalCompare); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::RelationalCompare); return Ok(); } // Only primitive (not double/string) or objects are supported. // I.e. Undefined/Null/Boolean/Int32/Symbol and Object if (!ObjectOrSimplePrimitive(left) || !ObjectOrSimplePrimitive(right)) { - trackOptimizationOutcome(TrackedOutcome::OperandTypeNotBitwiseComparable); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::OperandTypeNotBitwiseComparable); return Ok(); } @@ -5933,7 +5953,8 @@ IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefini if (left->maybeEmulatesUndefined(constraints()) || right->maybeEmulatesUndefined(constraints())) { - trackOptimizationOutcome(TrackedOutcome::OperandMaybeEmulatesUndefined); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::OperandMaybeEmulatesUndefined); return Ok(); } @@ -5946,7 +5967,8 @@ IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefini if ((left->mightBeType(MIRType::Undefined) && right->mightBeType(MIRType::Null)) || (left->mightBeType(MIRType::Null) && right->mightBeType(MIRType::Undefined))) { - trackOptimizationOutcome(TrackedOutcome::LoosyUndefinedNullCompare); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::LoosyUndefinedNullCompare); return Ok(); } @@ -5955,7 +5977,8 @@ IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefini if ((left->mightBeType(MIRType::Int32) && right->mightBeType(MIRType::Boolean)) || (left->mightBeType(MIRType::Boolean) && right->mightBeType(MIRType::Int32))) { - trackOptimizationOutcome(TrackedOutcome::LoosyInt32BooleanCompare); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::LoosyInt32BooleanCompare); return Ok(); } @@ -5970,7 +5993,8 @@ IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefini if ((left->mightBeType(MIRType::Object) && simpleRHS) || (right->mightBeType(MIRType::Object) && simpleLHS)) { - trackOptimizationOutcome(TrackedOutcome::CallsValueOf); + if (canTrackOptimization) + trackOptimizationOutcome(TrackedOutcome::CallsValueOf); return Ok(); } } @@ -5983,7 +6007,8 @@ IonBuilder::compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefini current->push(ins); MOZ_ASSERT(!ins->isEffectful()); - trackOptimizationSuccess(); + if (canTrackOptimization) + trackOptimizationSuccess(); *emitted = true; return Ok(); } @@ -5993,6 +6018,11 @@ IonBuilder::compareTrySpecializedOnBaselineInspector(bool* emitted, JSOp op, MDe MDefinition* right) { MOZ_ASSERT(*emitted == false); + + // Not supported for call expressions. + if (IsCallPC(pc)) + return Ok(); + trackOptimizationAttempt(TrackedStrategy::Compare_SpecializedOnBaselineTypes); // Try to specialize based on any baseline caches that have been generated @@ -6034,7 +6064,7 @@ IonBuilder::compareTryBinaryStub(bool* emitted, MDefinition* left, MDefinition* if (JitOptions.disableSharedStubs) return Ok(); - if (JSOp(*pc) == JSOP_CASE) + if (JSOp(*pc) == JSOP_CASE || IsCallPC(pc)) return Ok(); MBinaryCache* stub = MBinaryCache::New(alloc(), left, right); diff --git a/js/src/jit/IonBuilder.h b/js/src/jit/IonBuilder.h index 2dfd98ba1121..72891f6d1138 100644 --- a/js/src/jit/IonBuilder.h +++ b/js/src/jit/IonBuilder.h @@ -326,7 +326,7 @@ class IonBuilder // jsop_compare helpers. AbortReasonOr compareTrySpecialized(bool* emitted, JSOp op, MDefinition* left, - MDefinition* right, bool canTrackOptimization); + MDefinition* right); AbortReasonOr compareTryBitwise(bool* emitted, JSOp op, MDefinition* left, MDefinition* right); AbortReasonOr compareTrySpecializedOnBaselineInspector(bool* emitted, JSOp op, diff --git a/js/src/jit/MCallOptimize.cpp b/js/src/jit/MCallOptimize.cpp index 4bce74b0278b..1310f522c3bd 100644 --- a/js/src/jit/MCallOptimize.cpp +++ b/js/src/jit/MCallOptimize.cpp @@ -2488,19 +2488,26 @@ IonBuilder::inlineObjectIs(CallInfo& callInfo) MIRType leftType = left->type(); MIRType rightType = right->type(); + auto mightBeFloatingPointType = [](MDefinition* def) { + return def->mightBeType(MIRType::Double) || def->mightBeType(MIRType::Float32); + }; + bool strictEq; bool incompatibleTypes = false; if (leftType == rightType) { // We can only compare the arguments with strict-equals semantics if - // they aren't floating-point types or values. Otherwise we need to - // use MSameValue. - strictEq = !(IsFloatingPointType(leftType) || leftType == MIRType::Value); + // they aren't floating-point types or values which might be floating- + // point types. Otherwise we need to use MSameValue. + strictEq = leftType != MIRType::Value + ? !IsFloatingPointType(leftType) + : (!mightBeFloatingPointType(left) && !mightBeFloatingPointType(right)); } else if (leftType == MIRType::Value) { - // Also use strict-equals when comparing a value with a non-number. - strictEq = !IsNumberType(rightType); + // Also use strict-equals when comparing a value with a non-number or + // the value cannot be a floating-point type. + strictEq = !IsNumberType(rightType) || !mightBeFloatingPointType(left); } else if (rightType == MIRType::Value) { // Dual case to the previous one, only with reversed operands. - strictEq = !IsNumberType(leftType); + strictEq = !IsNumberType(leftType) || !mightBeFloatingPointType(right); } else if (IsNumberType(leftType) && IsNumberType(rightType)) { // Both arguments are numbers, but with different representations. We // can't use strict-equals semantics to compare the operands, but @@ -2513,23 +2520,18 @@ IonBuilder::inlineObjectIs(CallInfo& callInfo) if (incompatibleTypes) { // The result is always |false| when comparing incompatible types. pushConstant(BooleanValue(false)); + } else if (strictEq) { + // Specialize |Object.is(lhs, rhs)| as |lhs === rhs|. + MOZ_TRY(jsop_compare(JSOP_STRICTEQ, left, right)); } else { - bool emitted = false; - if (strictEq) { - // Specialize |Object.is(lhs, rhs)| as |lhs === rhs|. - MOZ_TRY(compareTrySpecialized(&emitted, JSOP_STRICTEQ, left, right, false)); - } + MSameValue* ins = MSameValue::New(alloc(), left, right); - if (!emitted) { - MSameValue* ins = MSameValue::New(alloc(), left, right); + // The more specific operand is expected to be in the rhs. + if (IsNumberType(leftType) && rightType == MIRType::Value) + ins->swapOperands(); - // The more specific operand is expected to be in the rhs. - if (IsNumberType(leftType) && rightType == MIRType::Value) - ins->swapOperands(); - - current->add(ins); - current->push(ins); - } + current->add(ins); + current->push(ins); } callInfo.setImplicitlyUsedUnchecked();