- 16 Sep, 2019 3 commits
-
-
Suraj Sharma authored
The new Smi handler created to handle StoreIC_Slow and KeyedStoreIC_Slow can get incorrectly assigned to global Objects. Added an extra Check to avoid that. Bug: chromium:1002628 Change-Id: I370e617e791792c98fa7b0cbf89ee7458f4e4c68 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803659Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Commit-Queue: Suraj Sharma <surshar@microsoft.com> Cr-Commit-Position: refs/heads/master@{#63813}
-
Bruce Dawson authored
For the last few months Chrome has been seeing many "impossible" crashes on Intel Gemini Lake, family 6 model 122 stepping 1 CPUs. These crashes only happen with 64-bit Chrome and only happen in the prologue of two functions. The crashes come and go across different Chrome versions. Analysis of most of the crashes shows that the address of the crashing instruction follows some patterns: When crashing in GetFieldIndex() the last byte of the address is always 1c, 5c, 9c, or dc. When crashing in UpdateCaches (fewer unique samples) the last byte of the address is always 5d or 9d. The address of the function is 0xc or 0xd bytes earlier so the crashing functions always start with an address that ends in 10, 50, 90, or d0. Those addresses are for the crashes on a load of the __security_cookie. The crashes also occasionally happen on the two instructions that follow the __security_cookie load in which case the crashing instruction's address has been seen to end with 23 or a3. This corresponds to a function start address of 10 or 90. Since the crash involves reading incorrect instruction bytes when crossing a 16-byte boundary and since the crash appears to only happen with particular 16-byte alignments it seems reasonable to force the function's alignments to a multiple of 32 to see if this reliably avoids the crashes. This change uses the gcc/clang __attribute__ directive to force 32-byte alignment. I have tested this change enough to verify that it triggers the desired alignment (with up to 31 "int 3" instructions added for padding) but since I have never reproduced this crash I have no way of testing its efficacy. Bug: chromium:968683, chromium:964273 Change-Id: Ia6e1c6d1e044b84d274817374b25523303e78b51 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803775Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Commit-Queue: Bruce Dawson <brucedawson@chromium.org> Cr-Commit-Position: refs/heads/master@{#63804}
-
Ross McIlroy authored
Also TNodify: - LoadJSPrimitiveWrapperValue - GetSuperConstructor - InstanceOf BUG=v8:6949,v8:9396 Change-Id: I4b39b418cdf01bd72e35441f037d16ede9c89ce9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803639 Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by:
Santiago Aboy Solanes <solanes@chromium.org> Cr-Commit-Position: refs/heads/master@{#63791}
-
- 13 Sep, 2019 4 commits
-
-
Igor Sheludko authored
... and from AccessorAssembler::LoadGlobalIC(). Tbr: tebbi@chromium.org Bug: v8:9708 Change-Id: Ia7d2ab30cfcfd513257655eb30a466d929ac774a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1801853 Commit-Queue: Igor Sheludko <ishell@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63759}
-
Ross McIlroy authored
Also TNodify TypeOf in code-stub-assembler. BUG=v8:6949, v8:9396 Change-Id: I12a66a077fe82df44ce3c46a87f1fda754be9423 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803334Reviewed-by:
Mythri Alle <mythria@chromium.org> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#63757}
-
Leszek Swirski authored
For minified files especially, the line number alone isn't enough to identify an IC site. Change-Id: I93f54f8fca1002072af0d702c155768fa2a8dbcb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1800566Reviewed-by:
Igor Sheludko <ishell@chromium.org> Reviewed-by:
Peter Marshall <petermarshall@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Auto-Submit: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63745}
-
Igor Sheludko authored
Bug: v8:9708 Change-Id: I91e429e478ad70dc2212f9f78830d10941fa47e6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1800581Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Commit-Queue: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#63740}
-
- 12 Sep, 2019 2 commits
-
-
Igor Sheludko authored
Bug: v8:9708 Change-Id: I305cc007a4e7302c8587b999cbb11f23ced4cfd3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1800579 Commit-Queue: Igor Sheludko <ishell@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63735}
-
Tobias Tebbi authored
This enables using TNode types without including code-assembler.h, which is useful when generating CallInterfaceDescriptors. As a drive-by, this moves TNode from v8::internal::compiler to v8::internal. It's only used outside of the compiler anyway. Change-Id: I3d938c22366a3570315041683094f77b0d1096a2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798425 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Michael Stanton <mvstanton@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63721}
-
- 11 Sep, 2019 1 commit
-
-
Santiago Aboy Solanes authored
functionality is: If rhs_is_smi is true, we are sure that rhs is a Smi. If rhs_is_smi is false, rhs might or not be a Smi. Therefore, rhs_known_smi fits better. Change-Id: Ie6dd0446ef85ba0730189e2012a21c24d1731b74 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1796551Reviewed-by:
Mythri Alle <mythria@chromium.org> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Cr-Commit-Position: refs/heads/master@{#63669}
-
- 10 Sep, 2019 4 commits
-
-
Santiago Aboy Solanes authored
Bug: v8:6949, v8:9396 Change-Id: I4c9382079190379661a26fbe6e1f4f6040a56d08 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1792902 Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#63658}
-
Leszek Swirski authored
Rather than duplicating code paths for in- and out-of-object stores, have one code path which checks whether it needs to load the property store (and change the storage location to the HeapNumber value for unboxed doubles). As a drive-by, change the representation dispatch into a switch, and inline the representation checks into that switch, to make explicit what checks for what and which paths transform the value. Also, TNodify some of the surrounding functions. Change-Id: Ia1bf698b4cec3ffce9aaa5732cda2e3be9efd8e8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1795345Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63652}
-
Suraj Sharma authored
based on dicussion at docs.google.com/document/d/1UzCOai9H07fYcSaSqvF_H7BS2-sF5q91A4r9O1mRnHc/ Bug: v8:9305 Change-Id: I7464d4267b6465cc02bc27dffb602c8871d846f9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1696285 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#63640}
-
Mythri A authored
We don't handle all cases for stores to typed arrays in the builtins related to storing a property. Bailout to runtime when storing into a typed array if the property is not found on the object. Bug: chromium:996161 Change-Id: I684c7c4f526b15cdfb5bfe3fd23218910486a59e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789396 Commit-Queue: Mythri Alle <mythria@chromium.org> Reviewed-by:
Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#63639}
-
- 09 Sep, 2019 1 commit
-
-
Santiago Aboy Solanes authored
TNodify: * FloatOp * BigIntOp * Loads into their respective types * return type of: * GetContextAtDepth * ConstructWithSpread * Construct * CallBuiltin Also TNodify CheckEnumCache in code-stub-assembler. Bug: v8:6949, v8:9396 Change-Id: I79a90296b4851e47f4b89ed52fadfc9b61be1e6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789161 Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#63625}
-
- 05 Sep, 2019 1 commit
-
-
Leszek Swirski authored
This is a reland of 981aafaf It adds double checks to LoadFieldByIndex in the optimizing compiler, which are likely the source of the crashes. Original change's description: > Reland "[ic] In-place Double -> Tagged transitions" > > This is a reland of 0736599a. > This is a reland of 7e1fbe8f. > > Original change description: > > [ic] In-place Double -> Tagged transitions > > > > With no more MutableHeapNumber, we can make Double -> Tagged transitions > > in-place, at the cost of an extra map check when accessing double fields > > to make sure they are still doubles. > > > > Bug: v8:9606 > > Change-Id: I74ff39ed6fba62ee223cd37dfe761f7d73020e1c > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743973 > > Reviewed-by: Tobias Tebbi <tebbi@chromium.org> > > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#63374} > > TBR=verwaest@chromium.org, tebbi@chromium.org > > Bug: v8:9606 > Change-Id: I2d1b7416064d743582f4983fb868316b7e8a4cf2 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1777661 > Reviewed-by: Leszek Swirski <leszeks@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63499} TBR=verwaest@chromium.org Bug: v8:9606 Bug: chromium:997989 Change-Id: Iccfff8e5c6306c9ee4f6c62767dce883b1c6f743 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1784288Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Auto-Submit: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63582}
-
- 04 Sep, 2019 2 commits
-
-
Tobias Tebbi authored
This reverts commit 352a154e. Reason for revert: https://crbug.com/999972 Original change's description: > [compiler] improve inlining heuristics: call frequency per executed bytecodes > > TLDR: Inline less, but more where it matters. ~10% decrease in Turbofan > compile time including off-thread, while improving Octane scores by ~2%. > > How things used to work: > > There is a flag FLAG_min_inlining_frequency that limits inlining by > the callsite being sufficiently frequently executed. This call frequency > was measured relative to invocations of the parent (= the function we > originally optimize). At the same time, the limit was very low (0.15), > meaning we mostly relied on the total amount of inlined code > (FLAG_max_inlined_bytecode_size_cumulative) to limit inlining. > > How things work now: > > Instead of measuring call frequency relative to parent invocations, we > should have a measure that predicts how often the callsite in question > will be executed in the future. An obvious attempt at that would be to > measure how often the callsite was executed in absolute numbers in the > past. But depending on how fast feedback stabilizes, it can take more > or less time until we optimize a function. If we just take the absolute > call frequency up to the point in time when we optimize, we would > inline more for functions that stabilize slowly, which doesn't make > sense. So instead, we measure absolute call count per KB of executed > bytecodes of the parent function. > Since inlining big functions is more expensive, this threshold is > additionally scaled linearly with the bytecode-size of the inlinee. > The resulting formula is: > call_frequency > > FLAG_min_inlining_frequency * > (bytecode.length() - FLAG_max_inlined_bytecode_size_small) / > (FLAG_max_inlined_bytecode_size - FLAG_max_inlined_bytecode_size_small) > > The new threshold is chosen in a way that it effectively limits > inlining, which allows us to increase > FLAG_max_inlined_bytecode_size_cumulative without increasing inlining > in general. > > The reduction in compile time (x64 build) of ~10% was observed in Octane, > ARES-6, web-tooling-benchmark, and the standalone TypeScript benchmark. > The hope is that this will reduce CPU-time in real-world situations > too. > The Octane improvements come from inlining more in places where it > matters. > > Bug: v8:6682 > > Change-Id: I99baa17dec85b71616a3ab3414d7e055beca39a0 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768366 > Commit-Queue: Tobias Tebbi <tebbi@chromium.org> > Reviewed-by: Jakob Gruber <jgruber@chromium.org> > Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63449} TBR=rmcilroy@chromium.org,neis@chromium.org,jgruber@chromium.org,tebbi@chromium.org,mslekova@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:6682 chromium:999972 Change-Id: Iffca63d4bef81afa0f66e34d35fb72f3b5baf517 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1784281Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#63554}
-
Leszek Swirski authored
This reverts commit 981aafaf. Reason for revert: Still crashing on Canary. Original change's description: > Reland "[ic] In-place Double -> Tagged transitions" > > This is a reland of 0736599a. > This is a reland of 7e1fbe8f. > > Original change description: > > [ic] In-place Double -> Tagged transitions > > > > With no more MutableHeapNumber, we can make Double -> Tagged transitions > > in-place, at the cost of an extra map check when accessing double fields > > to make sure they are still doubles. > > > > Bug: v8:9606 > > Change-Id: I74ff39ed6fba62ee223cd37dfe761f7d73020e1c > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743973 > > Reviewed-by: Tobias Tebbi <tebbi@chromium.org> > > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#63374} > > TBR=verwaest@chromium.org, tebbi@chromium.org > > Bug: v8:9606 > Change-Id: I2d1b7416064d743582f4983fb868316b7e8a4cf2 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1777661 > Reviewed-by: Leszek Swirski <leszeks@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63499} TBR=leszeks@chromium.org, verwaest@chromium.org, tebbi@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:9606 Bug: chromium:997989 Change-Id: Ic95166e67df68e84a524dffd8155121c3ff6aa13 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1784283 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63550}
-
- 02 Sep, 2019 1 commit
-
-
Leszek Swirski authored
This is a reland of 0736599a. This is a reland of 7e1fbe8f. Original change description: > [ic] In-place Double -> Tagged transitions > > With no more MutableHeapNumber, we can make Double -> Tagged transitions > in-place, at the cost of an extra map check when accessing double fields > to make sure they are still doubles. > > Bug: v8:9606 > Change-Id: I74ff39ed6fba62ee223cd37dfe761f7d73020e1c > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743973 > Reviewed-by: Tobias Tebbi <tebbi@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63374} TBR=verwaest@chromium.org, tebbi@chromium.org Bug: v8:9606 Change-Id: I2d1b7416064d743582f4983fb868316b7e8a4cf2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1777661Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63499}
-
- 30 Aug, 2019 3 commits
-
-
Sathya Gunasekaran authored
This reverts commit f6e08f43. This patch doesn't allow thin/cons strings to be inlined as weak refs to them are not supported by the GC. Bug: v8:9616 Change-Id: I0407654bd9d20fe0182de4b8554e21ddbce8b28c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1774720 Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Reviewed-by:
Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#63478}
-
Leszek Swirski authored
This is a reland of 1fba0441 Chromium expectation tests have been disabled, and will be enabled Original change's description: > [destructuring] Elide coercible check for simple keys > > Simple object destructuring, such as `let {a,b} = o`, is less efficient > than the equivalent assignments `let a = o.a; let b = o.b`. This is > because it does a nil check of `o` before the assignments. However, this > nil check is not strictly necessary for simple (i.e. non-computed) names, > as there will be an equivalent nil check on the first access to o in > `o.a`. For computed names the computation is unfortunately obervable. > > So, we can elide the nil check when the first property (if any) of the > destructuring target is a non-computed name. This messes a bit with our > error messages, so we re-use the CallPrinter to also find destructuring > assignment based errors, and fiddle with the error message there. As > a side-effect, we also get out the object name in the AST, so we can > output a slightly nicer error message. > > Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254 > Reviewed-by: Leszek Swirski <leszeks@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63453} TBR=verwaest@chromium.org Bug: chromium:999473 Change-Id: Ib0b2e4be433c50521ba1722e1c06b672bfefa405 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1777702Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63477}
-
Joyee Cheung authored
This patch implements the access of private accessors by loading the referenced component from the AccessorPair associated with private name variables. It also makes the error messages for invalid kind of private accessor access more specific. Bug: v8:8330 Design doc: https://docs.google.com/document/d/10W4begYfs7lmldSqBoQBBt_BKamgT8igqxF9u50RGrI/edit Change-Id: I6d441cffb85f8d9cd0417ec9b6ae20f3e34ef418 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1695205Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/master@{#63474}
-
- 29 Aug, 2019 4 commits
-
-
Adam Klein authored
This reverts commit 1fba0441. Reason for revert: blocks V8 roll due to layout test failures caused by error message changes: https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Linux/347 Original change's description: > [destructuring] Elide coercible check for simple keys > > Simple object destructuring, such as `let {a,b} = o`, is less efficient > than the equivalent assignments `let a = o.a; let b = o.b`. This is > because it does a nil check of `o` before the assignments. However, this > nil check is not strictly necessary for simple (i.e. non-computed) names, > as there will be an equivalent nil check on the first access to o in > `o.a`. For computed names the computation is unfortunately obervable. > > So, we can elide the nil check when the first property (if any) of the > destructuring target is a non-computed name. This messes a bit with our > error messages, so we re-use the CallPrinter to also find destructuring > assignment based errors, and fiddle with the error message there. As > a side-effect, we also get out the object name in the AST, so we can > output a slightly nicer error message. > > Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254 > Reviewed-by: Leszek Swirski <leszeks@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63453} TBR=leszeks@chromium.org,verwaest@chromium.org Change-Id: I74cf06ebd987e5b8bbe1831b0042c085edf37f5b No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1776994Reviewed-by:
Adam Klein <adamk@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#63465}
-
Leszek Swirski authored
Simple object destructuring, such as `let {a,b} = o`, is less efficient than the equivalent assignments `let a = o.a; let b = o.b`. This is because it does a nil check of `o` before the assignments. However, this nil check is not strictly necessary for simple (i.e. non-computed) names, as there will be an equivalent nil check on the first access to o in `o.a`. For computed names the computation is unfortunately obervable. So, we can elide the nil check when the first property (if any) of the destructuring target is a non-computed name. This messes a bit with our error messages, so we re-use the CallPrinter to also find destructuring assignment based errors, and fiddle with the error message there. As a side-effect, we also get out the object name in the AST, so we can output a slightly nicer error message. Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63453}
-
Tobias Tebbi authored
TLDR: Inline less, but more where it matters. ~10% decrease in Turbofan compile time including off-thread, while improving Octane scores by ~2%. How things used to work: There is a flag FLAG_min_inlining_frequency that limits inlining by the callsite being sufficiently frequently executed. This call frequency was measured relative to invocations of the parent (= the function we originally optimize). At the same time, the limit was very low (0.15), meaning we mostly relied on the total amount of inlined code (FLAG_max_inlined_bytecode_size_cumulative) to limit inlining. How things work now: Instead of measuring call frequency relative to parent invocations, we should have a measure that predicts how often the callsite in question will be executed in the future. An obvious attempt at that would be to measure how often the callsite was executed in absolute numbers in the past. But depending on how fast feedback stabilizes, it can take more or less time until we optimize a function. If we just take the absolute call frequency up to the point in time when we optimize, we would inline more for functions that stabilize slowly, which doesn't make sense. So instead, we measure absolute call count per KB of executed bytecodes of the parent function. Since inlining big functions is more expensive, this threshold is additionally scaled linearly with the bytecode-size of the inlinee. The resulting formula is: call_frequency > FLAG_min_inlining_frequency * (bytecode.length() - FLAG_max_inlined_bytecode_size_small) / (FLAG_max_inlined_bytecode_size - FLAG_max_inlined_bytecode_size_small) The new threshold is chosen in a way that it effectively limits inlining, which allows us to increase FLAG_max_inlined_bytecode_size_cumulative without increasing inlining in general. The reduction in compile time (x64 build) of ~10% was observed in Octane, ARES-6, web-tooling-benchmark, and the standalone TypeScript benchmark. The hope is that this will reduce CPU-time in real-world situations too. The Octane improvements come from inlining more in places where it matters. Bug: v8:6682 Change-Id: I99baa17dec85b71616a3ab3414d7e055beca39a0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768366 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Jakob Gruber <jgruber@chromium.org> Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Reviewed-by:
Georg Neis <neis@chromium.org> Reviewed-by:
Maya Lekova <mslekova@chromium.org> Cr-Commit-Position: refs/heads/master@{#63449}
-
Leszek Swirski authored
This reverts commit 0736599a. This reverts commit 7e1fbe8f. Reason for revert: Still some crashes, reverting to unblock dev. TBR=ishell@chromium.org,tebbi@chromium.org Bug: v8:9606 Bug: chromium:997485 Bug: chromium:997989 Change-Id: I9a0cb5440bf4fce06c9e6134dacf5c03d512f049 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773271 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63441}
-
- 28 Aug, 2019 1 commit
-
-
Z Nguyen-Huu authored
Currently the backing store and elements kind might not aligned aka backing store can be dictionary where elements kind is frozen/sealed element kinds or the other way around. The reason is that Object.preventExtensions change elements kind to DICTIONARY while Object.seal/freeze change elements kind to SEALED/FROZEN element kind. Apply both these operations can lead to that problem as in chromium:992914 To solve this issue, we avoid Object.preventExtensions to change backing store to dictionary by introducing new nonextensible elements kind. These new nonextensible elements kind are handled similar to frozen, sealed element kinds. This change not only fixes the problem but also optimize the performance of nonextensible objects. Change-Id: Iffc7f14eb48223c11abf3c577f305d2d072eb65b Bug: chromium:992914, v8:6831 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1760976 Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#63432}
-
- 27 Aug, 2019 1 commit
-
-
Leszek Swirski authored
Using the tool again, the previous iteration accidentally ignored Node/TNode behind a typedef. Automatic replacement of types with manual cleanup/addition of CASTs where necessary. Bug: v8:9396 Change-Id: I33b6d229669cb80586d5d8e82c04542df671f0b9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768367 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#63409}
-
- 26 Aug, 2019 2 commits
-
-
Leszek Swirski authored
For stores with Double feedback, StoreIC needs to check that the representation is still Double before doing the store, in case it accidentally tries to write to an object or worse, mutate a non-mutable HeapNumber. Bug: v8:9606 Bug: chromium:997485 Change-Id: I51e0953b40f752648c5e86b8644c23baf636367e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768373 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#63402}
-
Leszek Swirski authored
Forbid using LoadRoot in CSA (with a bailout via CodeAssembler), so that users are forced to use helper macros for roots, which have statically known types. Convert all current uses of LoadRoot to use these macros, introducing new ones where necessary. Bug: v8:9396 Change-Id: I91214fca6e5ace7554d79605706a8a60117468fa Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1762526 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#63398}
-
- 23 Aug, 2019 2 commits
-
-
Leszek Swirski authored
With no more MutableHeapNumber, we can make Double -> Tagged transitions in-place, at the cost of an extra map check when accessing double fields to make sure they are still doubles. Bug: v8:9606 Change-Id: I74ff39ed6fba62ee223cd37dfe761f7d73020e1c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743973Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#63374}
-
Leszek Swirski authored
Replace uses of WordEqual on two tagged representation nodes with a new TaggedEqual helper, which on pointer compressed configs only compares the bottom 32-bits of the word. We no longer allow using WordEqual on anything not known to be a WordT (i.e. Node* or TNode<Object>). In the future, this may allow us to ignore the top bits of an uncompressed Smi, and have simpler decompression, though this patch is not sufficient for such a change. As a necessary drive-by, TNodify a bunch of stuff. Bug: v8:8948 Change-Id: Ie11b70709e5d3073f12551b37b420a172a71bc99 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1763531 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Tobias Tebbi <tebbi@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Reviewed-by:
Santiago Aboy Solanes <solanes@chromium.org> Cr-Commit-Position: refs/heads/master@{#63372}
-
- 22 Aug, 2019 3 commits
-
-
Bill Budge authored
This reverts commit 8ee507f1. Reason for revert: Speculative, to unblock the V8 roller https://ci.chromium.org/p/chromium/builders/try/linux-rel/173637 Original change's description: > [ic] Inline constant fields in IC > > Previously, the handler would load the constant field from the holder > everytime by using the descriptor index. Instead, this patch inlines > the constant field directly into the handler. > > Change-Id: Ia731811b135897033f4c5dc973031a30f25a64ed > Bug: v8:9616 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1688829 > Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63332} TBR=gsathya@chromium.org,ishell@chromium.org,verwaest@chromium.org Change-Id: I36c5648c56f1d78447b7a45504cdebf593c020a1 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:9616 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1766148Reviewed-by:
Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#63353}
-
Sathya Gunasekaran authored
This reverts commit 5c59ba4f. Reason for revert: requires more thinking Original change's description: > [ic] Fix KeyedLoadIC for ArrayIndex access > > Previously, without support for converting strings to numbers we'd > switch to megamorphic state and go to the runtime always to do the > conversion causing a performance cliff. > > This patch improves the following js-perf-test scores: > Object-Lookup-String-Constant-BytecodeHandler: 4.25% > Object-Lookup-Index-String-BytecodeHandler: 5.41% > > Bug: v8:9449 > Change-Id: I63787fa84373fc946f1304b0141e48a52a1b4bcb > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1690953 > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63293} TBR=mythria@chromium.org,jyan@ca.ibm.com,gsathya@chromium.org,leszeks@chromium.org,ishell@chromium.org,verwaest@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:9449 Change-Id: I6b6ad5901175c2e6bbd7516b13e91471adb5776d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1765532Reviewed-by:
Sathya Gunasekaran <gsathya@chromium.org> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#63347}
-
Sathya Gunasekaran authored
Previously, the handler would load the constant field from the holder everytime by using the descriptor index. Instead, this patch inlines the constant field directly into the handler. Change-Id: Ia731811b135897033f4c5dc973031a30f25a64ed Bug: v8:9616 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1688829 Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by:
Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#63332}
-
- 20 Aug, 2019 2 commits
-
-
Leszek Swirski authored
Since the mutability of HeapNumbers is determined by their owning object's descriptor array, we can remove the MutableHeapNumber type entirely, at the cost of a few fewer DCHECKs and a couple of TODOs to use the descriptor array information. This is a necessary step towards a follow-up which allows in-place Double -> Tagged transitions Design doc: https://docs.google.com/document/d/1VeKIskAakxQFnUBNkhBmVswgR7Vk6T1kAyKRLhqerb4/ Bug: v8:9606 Change-Id: I13209f9c86f1f204088f6fd80089e17d956b4a50 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1743972 Commit-Queue: Leszek Swirski <leszeks@chromium.org> Reviewed-by:
Ulan Degenbaev <ulan@chromium.org> Reviewed-by:
Michael Starzinger <mstarzinger@chromium.org> Reviewed-by:
Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#63294}
-
Sathya Gunasekaran authored
Previously, without support for converting strings to numbers we'd switch to megamorphic state and go to the runtime always to do the conversion causing a performance cliff. This patch improves the following js-perf-test scores: Object-Lookup-String-Constant-BytecodeHandler: 4.25% Object-Lookup-Index-String-BytecodeHandler: 5.41% Bug: v8:9449 Change-Id: I63787fa84373fc946f1304b0141e48a52a1b4bcb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1690953Reviewed-by:
Igor Sheludko <ishell@chromium.org> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#63293}
-
- 19 Aug, 2019 2 commits
-
-
Santiago Aboy Solanes authored
This is a reland of 82111e22 Relanding since we now have more shards: https://chromium-review.googlesource.com/c/v8/v8/+/1760810 Original change's description: > [CSA][cleanup] TNodify some methods related to prototype and property lookup > > This is a CL in a string of CLs that aims to TNodify CSA. In particular, > there were some loads that were done in AnyTagged instead of > TaggedPointer. TNode-ifying them brings improvement in pointer > compression since we are able to decompress using the Pointer > decompression. > > TNodified: > * LoadJSFunctionPrototype > * TryPrototypeChainLookup > * OrdinaryHasInstance > > Also TNodified loads regarding: > * FeedbackCell::kValueOffset > * HeapObject::kMapOffset > * JSFunction::kSharedFunctionInfoOffset > * JSFunction::kFeedbackCellOffset > * Map::kInstanceTypeOffset > * Map::kInstanceDescriptorsOffset > * Map::kPrototypeOffset > > Drive-by cleanup: StoreJSArrayLength and StoreElements were unused. > > Bug: v8:6949, v8:9396 > Change-Id: I89697b5c02490906be1eee63cf3d9e60a1094d48 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1755844 > Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> > Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63216} Bug: v8:6949, v8:9396 Change-Id: I040aefcf8af60611f7b3c24f3bd5c661e03b6ada Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1760811Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Cr-Commit-Position: refs/heads/master@{#63249}
-
Maya Lekova authored
This reverts commit 82111e22. Reason for revert: Speculative revert, could be causing timeouts - https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20arm%20-%20sim%20-%20debug/17567 Original change's description: > [CSA][cleanup] TNodify some methods related to prototype and property lookup > > This is a CL in a string of CLs that aims to TNodify CSA. In particular, > there were some loads that were done in AnyTagged instead of > TaggedPointer. TNode-ifying them brings improvement in pointer > compression since we are able to decompress using the Pointer > decompression. > > TNodified: > * LoadJSFunctionPrototype > * TryPrototypeChainLookup > * OrdinaryHasInstance > > Also TNodified loads regarding: > * FeedbackCell::kValueOffset > * HeapObject::kMapOffset > * JSFunction::kSharedFunctionInfoOffset > * JSFunction::kFeedbackCellOffset > * Map::kInstanceTypeOffset > * Map::kInstanceDescriptorsOffset > * Map::kPrototypeOffset > > Drive-by cleanup: StoreJSArrayLength and StoreElements were unused. > > Bug: v8:6949, v8:9396 > Change-Id: I89697b5c02490906be1eee63cf3d9e60a1094d48 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1755844 > Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> > Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#63216} TBR=rmcilroy@chromium.org,solanes@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:6949, v8:9396 Change-Id: Ib6ae8fe86a598ed1066894595565e1162cf7dd1f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1758310Reviewed-by:
Maya Lekova <mslekova@chromium.org> Reviewed-by:
Santiago Aboy Solanes <solanes@chromium.org> Commit-Queue: Maya Lekova <mslekova@chromium.org> Cr-Commit-Position: refs/heads/master@{#63233}
-
- 15 Aug, 2019 1 commit
-
-
Santiago Aboy Solanes authored
This is a CL in a string of CLs that aims to TNodify CSA. In particular, there were some loads that were done in AnyTagged instead of TaggedPointer. TNode-ifying them brings improvement in pointer compression since we are able to decompress using the Pointer decompression. TNodified: * LoadJSFunctionPrototype * TryPrototypeChainLookup * OrdinaryHasInstance Also TNodified loads regarding: * FeedbackCell::kValueOffset * HeapObject::kMapOffset * JSFunction::kSharedFunctionInfoOffset * JSFunction::kFeedbackCellOffset * Map::kInstanceTypeOffset * Map::kInstanceDescriptorsOffset * Map::kPrototypeOffset Drive-by cleanup: StoreJSArrayLength and StoreElements were unused. Bug: v8:6949, v8:9396 Change-Id: I89697b5c02490906be1eee63cf3d9e60a1094d48 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1755844 Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Reviewed-by:
Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#63216}
-