This does get a bit hairy, and I am curious if there is a better way to manage
the registers. See the comment in MacroAssembler.cpp.
Differential Revision: https://phabricator.services.mozilla.com/D175096
This moves the dynamic slot allocation code that is currently duplicated
between nursery and tenured allocation paths into NativeObject instead.
Differential Revision: https://phabricator.services.mozilla.com/D175373
This makes our JIT code for the megamorphic cache lookup a bit more compact by
caching the slot's offset + `isFixedSlot` flag in a single 32-bit value.
This improves some local micro-benchmarks a bit, but it's hard to see an improvement
on Speedometer. Maybe it improves some of the subtests a tiny bit and combined with
the shorter and simpler JIT code that might be good enough.
It's possible to go further and combine the load instruction for fixed vs dynamic,
and to replace the `isFixedSlot` branch with a CMOV instruction, but that seemed to
be a bit harder on the CPU (the branch is probably quite predictable in practice)
so this is the more conservative approach.
Differential Revision: https://phabricator.services.mozilla.com/D171532
So, this patch is a few things in one. The core idea is that we want to inline
megamorphic gets in Baseline, following the success of doing so in Ion.
However, Baseline misses the cache more often than Ion, so any extra costs
which are only incurred on cache miss are going to be more of an issue there.
Accordingly, this patch eliminates most of the overhead of inlining the cache
lookup by passing the cache entry pointer through the ABI call on miss,
allowing the C++ code to forgo the lookup.
Differential Revision: https://phabricator.services.mozilla.com/D169961
This patch reduces the time taken by the workload in bug 1814796 by about 10%.
This only checks the cache when called from optimised code. I didn't add the
check for baseline or cache IR.
Differential Revision: https://phabricator.services.mozilla.com/D168825
The `Shape` methods related to property information and number of fixed slots are
moved into `NativeShape`, to improve type safety.
Differential Revision: https://phabricator.services.mozilla.com/D164213
We can't inline `String.prototype.charAt` and `String.prototype.charCodeAt` in a
couple of JetStream tests because the input is a nested rope.
Before this change, we can observe the following number of non-inlined calls to
these functions:
crypto
String.prototype.charCodeAt 360'000
pdfjs
String.prototype.charCodeAt 21'910'000
typescript
String.prototype.charCodeAt 17'840'000
base64-SP
String.prototype.charCodeAt 39'300'000
crypto-md5-SP
String.prototype.charCodeAt 41'750'000
crypto-sha1-SP
String.prototype.charCodeAt 30'180'000
acorn-wtb
String.prototype.charCodeAt 3'370'000
jshint-wtb
String.prototype.charAt 2'450'000
String.prototype.charCodeAt 200'000
uglify-js-wtb
String.prototype.charAt 450'000
After this change, only the WBT sub-benchmarks still have non-inlined calls:
acorn-wtb
String.prototype.charCodeAt 1'150'000
jshint-wtb
String.prototype.charAt 2'450'000
String.prototype.charCodeAt 200'000
uglify-js-wtb
String.prototype.charAt 450'000
Differential Revision: https://phabricator.services.mozilla.com/D162726
The existing implementation of StoreElementHole could handle indices > length, but we never actually used that path, because the corresponding code in `CacheIRCompiler::emitStoreDenseElementHole` only handles the index == initLength case (where we are adding one past the end). By aligning the Ion code more closely with the CacheIr implementation, we can remove some complexity. In particular, bailing out when index != length handles negative indices for free, which lets us remove the `needsNegativeIntCheck` code.
Differential Revision: https://phabricator.services.mozilla.com/D163280
In addition to appeasing Coverity, this also fixes the `MOZ_ASSERT_IF(JitOptions.enableWatchtowerMegamorphic, entry)` in `GetNativeDataPropertyPureFallback`, which clearly expects `entry` to be null if megamorphic watchtower is disabled.
Differential Revision: https://phabricator.services.mozilla.com/D158855
This inlines the megamorphic cache lookup, netting us about a 30-40%
improvement for the microbenchmark on top of the prior optimizations. There is
no measured performance losses for speedometer or matrix-react, and maybe a
1% win overall, though I need to do more runs with the cleaned up patch to be
confident.
We could do similar inlining for the CacheIRCompiler version and for
MegamorphicLoadSlotByValue - however, we wouldn't be able to include the id and
id hash as immediates, which would mean we'd need another register and several
more instructions to make it work, and it would be awkward to share the masm
code because of the different register requirements.
Differential Revision: https://phabricator.services.mozilla.com/D156615
Initially we explored caching this check on the shape, which still seems like
a decent idea. However, we can also just sidestep the problem for the most part
be moving the check after the cache lookup, since we can't get a cache hit if
it's not a NativeObject anyway, and we're hoping that a cache hit should be the
dominant path in most cases.
Differential Revision: https://phabricator.services.mozilla.com/D156614
Use a bailout instead of calling `DefineDataElement`. This won't lead to
performance issues, because calling `DefineDataElement` will add an indexed
property and indexed properties will cause shape guards to fail the next time
this operation is called.
That means we can also change the function to an ABI call, which is slightly
faster than a VM call. We need additional temp registers for the ABI call,
therefore change the Spectre-temp to a normal, unconditional temp register.
Differential Revision: https://phabricator.services.mozilla.com/D157548
More than 94% of calls to `SetElementMegamorphic` on matrix-react-bench, Speedometer 2,
and a number of popular websites can be optimized with this fast path.
This makes a micro-benchmark 1.72x faster and should eliminate most of the overhead,
but we could potentially optimize this more in the future with the megamorphic
cache.
Depends on D157331
Differential Revision: https://phabricator.services.mozilla.com/D157332
This is just nice to have at this point, but might make it a bit more clearer
what should happen when `setOrExtendDenseElements` returns `Incomplete`. For
example we don't want to execute a [[Set]] operation which might trigger
accessors on the prototype chain. (`array_push` uses [[Set]] internally.)
Depends on D157104
Differential Revision: https://phabricator.services.mozilla.com/D157105
Define has the correct semantics whether or not this is a PropInit operation
and it helps to avoid reintroducing bug 1607443.
Depends on D157102
Differential Revision: https://phabricator.services.mozilla.com/D157103
This matches CacheIR more closely and might allow to re-add the alias set which was
removed in bug 1607443.
Depends on D157101
Differential Revision: https://phabricator.services.mozilla.com/D157102
Some non-functional, renaming-style cleanups:
* TypedObject has been consistently renamed to WasmGcObject, both as a class
name and as file names.
* WasmGcObject.h (new name):
- removed unused IsWasm{Struct,Array}ObjectClass()
- removed unused JSObject::is<js::Wasm{Struct,Array}Object>()
* WasmGcObject.cpp (new name) has been re-sequenced to match its header file.
* TypedObject-inl.h has been removed. Its one-and-only method
::allocKindForRttValue() has been moved back to the "standard" header
(WasmGcObject.h). The only use point for that method is from an
expensive-looking method (caller) so there's no point in inlining it. Can
be reinstated later if profiling shows it is hot.
Differential Revision: https://phabricator.services.mozilla.com/D156045
Add emplace methods to JSString and BigInt types to reduce amount of C++
undefined-behaviour by running the constructor of the concrete derived type.
This is in contrast to previous behaviour of allocating a gc::Cell and then
simply casting to the final type. We still first allocate as a Cell from the GC
and then use placement-new to initialize as the correct type.
Differential Revision: https://phabricator.services.mozilla.com/D134095
This one was extra tricky, because removing includes from header files in
"js/src/vm" often leads to build errors in other files due to missing header
files which were previously only transitively included.
Differential Revision: https://phabricator.services.mozilla.com/D155342
This one was extra tricky, because removing includes from header files in
"js/src/vm" often leads to build errors in other files due to missing header
files which were previously only transitively included.
Differential Revision: https://phabricator.services.mozilla.com/D155342
The `argv` method on `BaselineFrame` does not include `this`, but the one on `JitFrameLayout` does.
We can be a bit more explicit about this.
Depends on D149962
Differential Revision: https://phabricator.services.mozilla.com/D149963
