From 44881811a825b6644a8a74e8e0666972c8dd364c Mon Sep 17 00:00:00 2001 From: Mike Stanton Date: Fri, 11 Jan 2019 10:46:27 +0100 Subject: [PATCH] [Torque] Modernize code style for Array.prototype.forEach Also fix an issue with naming in Array.prototype.filter that wasn't addressed before. Change-Id: I7465eda12e6981f46f6efa2efc81183cbdffea01 Reviewed-on: https://chromium-review.googlesource.com/c/1400847 Commit-Queue: Michael Stanton Reviewed-by: Michael Stanton Reviewed-by: Yang Guo Cr-Commit-Position: refs/heads/master@{#58728} --- src/builtins/array-filter.tq | 24 ++--- src/builtins/array-foreach.tq | 168 +++++++++++++++------------------- src/debug/debug-evaluate.cc | 1 + 3 files changed, 85 insertions(+), 108 deletions(-) diff --git a/src/builtins/array-filter.tq b/src/builtins/array-filter.tq index d7ddf8c6d9..222e4e291b 100644 --- a/src/builtins/array-filter.tq +++ b/src/builtins/array-filter.tq @@ -104,33 +104,33 @@ namespace array { thisArg: Object, a: JSArray) labels Bailout(Smi, Smi) { let k: Smi = 0; let to: Smi = 0; - const oFastWitness: FastJSArrayWitness = + const fastOWitness: FastJSArrayWitness = MakeWitness(Cast(o) otherwise goto Bailout(k, to)); - const aFastWitness: FastJSArrayWitness = + const fastAWitness: FastJSArrayWitness = MakeWitness(Cast(a) otherwise goto Bailout(k, to)); // Build a fast loop over the smi array. - for (; k < len; k = k + 1) { - let oFast: FastJSArray = - Testify(oFastWitness) otherwise goto Bailout(k, to); + for (; k < len; k++) { + let fastO: FastJSArray = + Testify(fastOWitness) otherwise goto Bailout(k, to); // Ensure that we haven't walked beyond a possibly updated length. - if (k >= oFast.length) goto Bailout(k, to); + if (k >= fastO.length) goto Bailout(k, to); try { const value: Object = - LoadElementNoHole(oFast, k) otherwise FoundHole; + LoadElementNoHole(fastO, k) otherwise FoundHole; const result: Object = - Call(context, callbackfn, thisArg, value, k, oFast); + Call(context, callbackfn, thisArg, value, k, fastO); if (ToBoolean(result)) { try { // Since the call to {callbackfn} is observable, we can't // use the Bailout label until we've successfully stored. // Hence the {SlowStore} label. - const aFast: FastJSArray = - Testify(aFastWitness) otherwise SlowStore; - if (aFast.length != to) goto SlowStore; - BuildAppendJSArray(kind, aFast, value) + const fastA: FastJSArray = + Testify(fastAWitness) otherwise SlowStore; + if (fastA.length != to) goto SlowStore; + BuildAppendJSArray(kind, fastA, value) otherwise SlowStore; } label SlowStore { diff --git a/src/builtins/array-foreach.tq b/src/builtins/array-foreach.tq index 7e33ff52cf..7967058e6b 100644 --- a/src/builtins/array-foreach.tq +++ b/src/builtins/array-foreach.tq @@ -3,9 +3,48 @@ // found in the LICENSE file. namespace array { - transitioning macro ArrayForEachTorqueContinuation( - context: Context, o: JSReceiver, len: Number, callbackfn: Callable, - thisArg: Object, initialK: Number): Object { + transitioning javascript builtin + ArrayForEachLoopEagerDeoptContinuation(implicit context: Context)( + receiver: Object, callback: Object, thisArg: Object, initialK: Object, + length: Object): Object { + // All continuation points in the optimized forEach implemntation are + // after the ToObject(O) call that ensures we are dealing with a + // JSReceiver. + const jsreceiver: JSReceiver = + Cast(receiver) otherwise unreachable; + const callbackfn: Callable = Cast(callback) otherwise unreachable; + const numberK: Number = Cast(initialK) otherwise unreachable; + const numberLength: Number = Cast(length) otherwise unreachable; + + return ArrayForEachLoopContinuation( + jsreceiver, callbackfn, thisArg, Undefined, jsreceiver, numberK, + numberLength, Undefined); + } + + transitioning javascript builtin + ArrayForEachLoopLazyDeoptContinuation(implicit context: Context)( + receiver: Object, callback: Object, thisArg: Object, initialK: Object, + length: Object, result: Object): Object { + // All continuation points in the optimized forEach implemntation are + // after the ToObject(O) call that ensures we are dealing with a + // JSReceiver. + const jsreceiver: JSReceiver = + Cast(receiver) otherwise unreachable; + const callbackfn: Callable = Cast(callback) otherwise unreachable; + const numberK: Number = Cast(initialK) otherwise unreachable; + const numberLength: Number = Cast(length) otherwise unreachable; + + return ArrayForEachLoopContinuation( + jsreceiver, callbackfn, thisArg, Undefined, jsreceiver, numberK, + numberLength, Undefined); + } + + transitioning builtin ArrayForEachLoopContinuation(implicit context: Context)( + receiver: JSReceiver, callbackfn: Callable, thisArg: Object, + array: Object, o: JSReceiver, initialK: Number, len: Number, + to: Object): Object { + // variables {array} and {to} are ignored. + // 5. Let k be 0. // 6. Repeat, while k < len for (let k: Number = initialK; k < len; k = k + 1) { @@ -30,106 +69,43 @@ namespace array { return Undefined; } - transitioning javascript builtin ArrayForEachLoopEagerDeoptContinuation( - context: Context, receiver: Object, callback: Object, thisArg: Object, - initialK: Object, length: Object): Object { - // The unsafe Cast is safe because all continuation points in forEach are - // after the ToObject(O) call that ensures we are dealing with a - // JSReceiver. - const jsreceiver: JSReceiver = UnsafeCast(receiver); - return ArrayForEachLoopContinuation( - context, jsreceiver, callback, thisArg, Undefined, jsreceiver, initialK, - length, Undefined); - } - - transitioning javascript builtin ArrayForEachLoopLazyDeoptContinuation( - context: Context, receiver: Object, callback: Object, thisArg: Object, - initialK: Object, length: Object, result: Object): Object { - // The unsafe Cast is safe because all continuation points in forEach are - // after the ToObject(O) call that ensures we are dealing with a - // JSReceiver. - const jsreceiver: JSReceiver = UnsafeCast(receiver); - return ArrayForEachLoopContinuation( - context, jsreceiver, callback, thisArg, Undefined, jsreceiver, initialK, - length, Undefined); - } - - transitioning builtin ArrayForEachLoopContinuation( - context: Context, receiver: JSReceiver, callback: Object, thisArg: Object, - array: Object, object: Object, initialK: Object, length: Object, - to: Object): Object { - try { - const callbackfn: Callable = - Cast(callback) otherwise Unexpected; - const k: Number = Cast(initialK) otherwise Unexpected; - const numberLength: Number = Cast(length) otherwise Unexpected; - - return ArrayForEachTorqueContinuation( - context, receiver, numberLength, callbackfn, thisArg, k); - } - label Unexpected deferred { - unreachable; - } - } - - transitioning macro VisitAllElements( - context: Context, a: JSArray, len: Smi, callbackfn: Callable, - thisArg: Object): void labels Bailout(Smi) { + transitioning macro VisitAllElements(implicit context: + Context)( + o: JSArray, len: Smi, callbackfn: Callable, thisArg: Object) labels + Bailout(Smi) { let k: Smi = 0; - const map: Map = a.map; + const fastOWitness: FastJSArrayWitness = + MakeWitness(Cast(o) otherwise goto Bailout(k)); - try { - // Build a fast loop over the smi array. - for (; k < len; k = k + 1) { - // Ensure that the map didn't change. - if (map != a.map) goto Slow; - // Ensure that we haven't walked beyond a possibly updated length. - if (k >= a.length) goto Slow; + // Build a fast loop over the smi array. + for (; k < len; k++) { + let fastO: FastJSArray = Testify(fastOWitness) otherwise goto Bailout(k); - try { - const value: Object = - LoadElementNoHole(a, k) otherwise FoundHole; - Call(context, callbackfn, thisArg, value, k, a); - } - label FoundHole { - // If we found the hole, we need to bail out if the initial - // array prototype has had elements inserted. This is preferable - // to walking the prototype chain looking for elements. + // Ensure that we haven't walked beyond a possibly updated length. + if (k >= fastO.length) goto Bailout(k); - if (IsNoElementsProtectorCellInvalid()) goto Bailout(k); - } + try { + const value: Object = + LoadElementNoHole(fastO, k) otherwise FoundHole; + Call(context, callbackfn, thisArg, value, k, fastO); } - } - label Slow deferred { - goto Bailout(k); + label FoundHole {} } } - transitioning macro FastArrayForEach( - context: Context, o: JSReceiver, len: Number, callbackfn: Callable, - thisArg: Object): Object + transitioning macro FastArrayForEach(implicit context: Context)( + o: JSReceiver, len: Number, callbackfn: Callable, thisArg: Object): Object labels Bailout(Smi) { let k: Smi = 0; - try { - const smiLen: Smi = Cast(len) otherwise Slow; - const a: JSArray = Cast(o) otherwise Slow; - const map: Map = a.map; - - if (!IsPrototypeInitialArrayPrototype(map)) goto Slow; - const elementsKind: ElementsKind = map.elements_kind; - if (!IsFastElementsKind(elementsKind)) goto Slow; - - if (IsElementsKindGreaterThan(elementsKind, HOLEY_ELEMENTS)) { - VisitAllElements( - context, a, smiLen, callbackfn, thisArg) - otherwise Bailout; - } else { - VisitAllElements(context, a, smiLen, callbackfn, thisArg) - otherwise Bailout; - } - } - label Slow deferred { - goto Bailout(k); + const smiLen: Smi = Cast(len) otherwise goto Bailout(k); + let fastO: FastJSArray = Cast(o) otherwise goto Bailout(k); + const elementsKind: ElementsKind = fastO.map.elements_kind; + if (IsElementsKindGreaterThan(elementsKind, HOLEY_ELEMENTS)) { + VisitAllElements(fastO, smiLen, callbackfn, thisArg) + otherwise Bailout; + } else { + VisitAllElements(fastO, smiLen, callbackfn, thisArg) + otherwise Bailout; } return Undefined; } @@ -161,15 +137,15 @@ namespace array { // Special cases. let k: Number = 0; try { - return FastArrayForEach(context, o, len, callbackfn, thisArg) + return FastArrayForEach(o, len, callbackfn, thisArg) otherwise Bailout; } label Bailout(kValue: Smi) deferred { k = kValue; } - return ArrayForEachTorqueContinuation( - context, o, len, callbackfn, thisArg, k); + return ArrayForEachLoopContinuation( + o, callbackfn, thisArg, Undefined, o, k, len, Undefined); } label TypeError deferred { ThrowTypeError(context, kCalledNonCallable, arguments[0]); diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc index c473d6acd8..cc2379d4d9 100644 --- a/src/debug/debug-evaluate.cc +++ b/src/debug/debug-evaluate.cc @@ -910,6 +910,7 @@ static bool TransitivelyCalledBuiltinHasNoSideEffect(Builtins::Name caller, case Builtins::kArrayFilterLoopContinuation: case Builtins::kArrayFindIndexLoopContinuation: case Builtins::kArrayFindLoopContinuation: + case Builtins::kArrayForEachLoopContinuation: case Builtins::kArrayIncludesHoleyDoubles: case Builtins::kArrayIncludesPackedDoubles: case Builtins::kArrayIncludesSmiOrObject: