New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 6
Cc:
Components:
HW: All
NextAction: ----
OS: All
Priority: 2
Type: Bug



Sign in to add a comment

Performance degradation when looping across character codes of a string

Project Member Reported by bmeu...@chromium.org, Jan 18

Issue description

Downstream issue: https://github.com/nodejs/node/issues/18218

It looks like with TurboFan we regressed String#charCodeAt performance. We already worked a bit on closing the gap, but there seems to be more to be done here, given that we tank performance on crc-32 significantly. I took the original repo and turned it into a simple performance test:

==============< bench-string-char-code-at.js >===================
if (typeof console === 'undefined') console = {log:print};

function foo() {
  var payload = "abcdefghijklmnopqrstuvwxyz";
  for(var j = 0; j < 12; ++j) payload += payload;

  var c = 0;
  for(j=payload.length-1; j >=0; --j) c+=payload.charCodeAt(j);
  return c;
}

const N = 1e3;

function test(fn) {
  var result;
  for (var i = 0; i < N; ++i) result = fn();
  return result;
}
test(foo);
test(foo);

var startTime = Date.now();
test(foo);
console.log('Time:', (Date.now() - startTime), 'ms.');
=================================================================

Running this with TurboFan vs. Crankshaft:

=================================================================
$ ./d8-master bench-string-char-code-at.js
Time: 654 ms.
$ ./d8-master --no-untrusted-code-mitigations bench-string-char-code-at.js
Time: 580 ms.
$ ./d8-5.8.283.38 bench-string-char-code-at.js
Time: 256 ms.
=================================================================

So there's roughly a 2.0x to 2.5x performance drop, which seem to be related to that way that TurboFan implements the String#charCodeAt compared to what Crankshaft did. One thing to do here would be to utilize the new builtin optimization guards to avoid the out-of-bounds handling in TurboFan as well, so the operation can never produce NaN if it's lowered.
 
Cc: petermarshall@chromium.org
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 18

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/db129b65259ff692a2675863a08f83f686c24029

commit db129b65259ff692a2675863a08f83f686c24029
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu Jan 18 08:27:35 2018

[turbofan] Speculate on bounds checks for String#charAt and String#charCodeAt.

With the new builtin optimization guard we can just speculatively assume
that the index passed to String#charAt and String#charCodeAt (in optimized
code) is going to be within the valid range for the receiver. This is
what Crankshaft used to do, and it avoids Smi checks on the result for
String#charCodeAt, since it can no longer return NaN.

This gives rise to further optimizations of these builtins (i.e. to
completely avoid the tagging of char codes), and by itself already
improves the regression test originally reported from 650ms to
610ms.

Bug:  v8:7127 ,  v8:7326 
Change-Id: Ia25a555c5c1a48d229c094b1ecd2487eec81e390
Reviewed-on: https://chromium-review.googlesource.com/872850
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50667}
[modify] https://crrev.com/db129b65259ff692a2675863a08f83f686c24029/src/compiler/js-call-reducer.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 18

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/27905a33f581c87fd47858eb71facab4691e8346

commit 27905a33f581c87fd47858eb71facab4691e8346
Author: Michael Hablich (vacation) <hablich@chromium.org>
Date: Thu Jan 18 11:34:17 2018

Revert "[turbofan] Speculate on bounds checks for String#charAt and String#charCodeAt."

This reverts commit db129b65259ff692a2675863a08f83f686c24029.

Reason for revert: blocks roll: https://chromium-review.googlesource.com/c/chromium/src/+/873150

Original change's description:
> [turbofan] Speculate on bounds checks for String#charAt and String#charCodeAt.
> 
> With the new builtin optimization guard we can just speculatively assume
> that the index passed to String#charAt and String#charCodeAt (in optimized
> code) is going to be within the valid range for the receiver. This is
> what Crankshaft used to do, and it avoids Smi checks on the result for
> String#charCodeAt, since it can no longer return NaN.
> 
> This gives rise to further optimizations of these builtins (i.e. to
> completely avoid the tagging of char codes), and by itself already
> improves the regression test originally reported from 650ms to
> 610ms.
> 
> Bug:  v8:7127 ,  v8:7326 
> Change-Id: Ia25a555c5c1a48d229c094b1ecd2487eec81e390
> Reviewed-on: https://chromium-review.googlesource.com/872850
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50667}

TBR=yangguo@chromium.org,bmeurer@chromium.org

Change-Id: I6d393a0797cac2fdfd67487a26ac1b178bd52813
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  v8:7127 ,  v8:7326 
Reviewed-on: https://chromium-review.googlesource.com/873355
Reviewed-by: Michael Hablich (vacation) <hablich@chromium.org>
Commit-Queue: Michael Hablich (vacation) <hablich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50672}
[modify] https://crrev.com/27905a33f581c87fd47858eb71facab4691e8346/src/compiler/js-call-reducer.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/93d67d20ee91c351091ef294e43e1352185b6d59

commit 93d67d20ee91c351091ef294e43e1352185b6d59
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Fri Jan 19 07:35:41 2018

[turbofan] Inline StringCharCodeAt like Crankshaft did.

This avoids the call to the StringCharCodeAt builtin from
within TurboFan optimized code and instead emits a loop
that does the character load. This (together with previously
reverted CL to the JSCallReducer) almost completely recovers
the performance regression caused when we shipped TurboFan.

Without untrusted code mitigations the benchmark goes from
580ms to roughly 490ms, and with the patch to the JSCallReducer
the time goes down to 280ms, which is very close to what we
had with Crankshaft.

This also renames the LoadFromString helper method in the
EffectControlLinearizer to LoadFromSeqString to make it
clear what it does.

Bug:  v8:7326 
Change-Id: Ibe0ec1847911a234f244bd8dcec6be18b241fda0
Reviewed-on: https://chromium-review.googlesource.com/873376
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50702}
[modify] https://crrev.com/93d67d20ee91c351091ef294e43e1352185b6d59/src/builtins/builtins-definitions.h
[modify] https://crrev.com/93d67d20ee91c351091ef294e43e1352185b6d59/src/builtins/builtins-string-gen.cc
[modify] https://crrev.com/93d67d20ee91c351091ef294e43e1352185b6d59/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/93d67d20ee91c351091ef294e43e1352185b6d59/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/93d67d20ee91c351091ef294e43e1352185b6d59/src/compiler/simplified-lowering.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/2fe91a91a764c576e668a0efa06111d4fc923e23

commit 2fe91a91a764c576e668a0efa06111d4fc923e23
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Jan 19 10:47:46 2018

Revert "[turbofan] Inline StringCharCodeAt like Crankshaft did."

This reverts commit 93d67d20ee91c351091ef294e43e1352185b6d59.

Reason for revert: several layout test failures:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/21062

Original change's description:
> [turbofan] Inline StringCharCodeAt like Crankshaft did.
> 
> This avoids the call to the StringCharCodeAt builtin from
> within TurboFan optimized code and instead emits a loop
> that does the character load. This (together with previously
> reverted CL to the JSCallReducer) almost completely recovers
> the performance regression caused when we shipped TurboFan.
> 
> Without untrusted code mitigations the benchmark goes from
> 580ms to roughly 490ms, and with the patch to the JSCallReducer
> the time goes down to 280ms, which is very close to what we
> had with Crankshaft.
> 
> This also renames the LoadFromString helper method in the
> EffectControlLinearizer to LoadFromSeqString to make it
> clear what it does.
> 
> Bug:  v8:7326 
> Change-Id: Ibe0ec1847911a234f244bd8dcec6be18b241fda0
> Reviewed-on: https://chromium-review.googlesource.com/873376
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50702}

TBR=yangguo@chromium.org,bmeurer@chromium.org

Change-Id: I6e909adba82adc46e269ab14426ee24caaca6ff9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  v8:7326 
Reviewed-on: https://chromium-review.googlesource.com/875963
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50709}
[modify] https://crrev.com/2fe91a91a764c576e668a0efa06111d4fc923e23/src/builtins/builtins-definitions.h
[modify] https://crrev.com/2fe91a91a764c576e668a0efa06111d4fc923e23/src/builtins/builtins-string-gen.cc
[modify] https://crrev.com/2fe91a91a764c576e668a0efa06111d4fc923e23/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/2fe91a91a764c576e668a0efa06111d4fc923e23/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/2fe91a91a764c576e668a0efa06111d4fc923e23/src/compiler/simplified-lowering.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c509d025c789e6357bd5de51a67dae0066f2c024

commit c509d025c789e6357bd5de51a67dae0066f2c024
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Fri Jan 19 15:16:47 2018

[turbofan] Inline StringCharCodeAt like Crankshaft did.

This avoids the call to the StringCharCodeAt builtin from
within TurboFan optimized code and instead emits a loop
that does the character load. This (together with previously
reverted CL to the JSCallReducer) almost completely recovers
the performance regression caused when we shipped TurboFan.

Without untrusted code mitigations the benchmark goes from
580ms to roughly 490ms, and with the patch to the JSCallReducer
the time goes down to 280ms, which is very close to what we
had with Crankshaft.

This also renames the LoadFromString helper method in the
EffectControlLinearizer to LoadFromSeqString to make it
clear what it does.

Bug:  v8:7326 
Change-Id: I6c77209ae01a3eacbd1e8fd40e4ad842eaf1999a
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/876102
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50725}
[modify] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/src/builtins/builtins-definitions.h
[modify] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/src/builtins/builtins-string-gen.cc
[modify] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/src/compiler/simplified-lowering.cc
[add] https://crrev.com/c509d025c789e6357bd5de51a67dae0066f2c024/test/mjsunit/string-charcodeat-external.js

Cc: -sigurds@chromium.org bmeu...@chromium.org
Owner: sigurds@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 24

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/90e50cc2ccd0bf858403d635cf0c264de2d60ea6

commit 90e50cc2ccd0bf858403d635cf0c264de2d60ea6
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Wed Jan 24 12:12:27 2018

[turbofan] Add effects to StringAt operators

Add effect input and output to String.p.char[Code]At/codePointAt.
This is necessary to fix an hard to reproduce bug, a repro for
which is included. However, the only way to get the repro
included in this CL to fail is to run it with the patch of

  873382:
  [turbofan] Speculate on bounds checks for String#char[Code]At

but WITHOUT this patch. This fixes a scheduling problem triggered
by 873382 that caused a bounds check to get scheduled after the
associated access.

Bug:  v8:7326 
Change-Id: I4b97c1726caac92ff8f74c23df2788f0ecfb1304
Reviewed-on: https://chromium-review.googlesource.com/881781
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50832}
[modify] https://crrev.com/90e50cc2ccd0bf858403d635cf0c264de2d60ea6/src/compiler/js-builtin-reducer.cc
[modify] https://crrev.com/90e50cc2ccd0bf858403d635cf0c264de2d60ea6/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/90e50cc2ccd0bf858403d635cf0c264de2d60ea6/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/90e50cc2ccd0bf858403d635cf0c264de2d60ea6/src/compiler/simplified-operator.cc
[add] https://crrev.com/90e50cc2ccd0bf858403d635cf0c264de2d60ea6/test/mjsunit/regress/regress-stringAt-boundsCheck.js

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 26

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ee2d85a37a06e4271afdaca8ffce008bed9103b8

commit ee2d85a37a06e4271afdaca8ffce008bed9103b8
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Fri Jan 26 12:00:58 2018

[turbofan] Speculate on bounds checks for String#char[Code]At

With the new builtin optimization guard we can just speculatively assume
that the index passed to String#charAt and String#charCodeAt (in
optimized
code) is going to be within the valid range for the receiver. This is
what Crankshaft used to do, and it avoids Smi checks on the result for
String#charCodeAt, since it can no longer return NaN.

This gives rise to further optimizations of these builtins (i.e. to
completely avoid the tagging of char codes), and by itself already
improves the regression test originally reported from 650ms to
610ms.

Bug:  v8:7127 ,  v8:7326 
Change-Id: I6c160540a1e002a37e44fa7f920e5e8f8c2c4210
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/873382
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50888}
[modify] https://crrev.com/ee2d85a37a06e4271afdaca8ffce008bed9103b8/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/ee2d85a37a06e4271afdaca8ffce008bed9103b8/src/compiler/js-call-reducer.h
[modify] https://crrev.com/ee2d85a37a06e4271afdaca8ffce008bed9103b8/test/mjsunit/constant-folding-2.js
[add] https://crrev.com/ee2d85a37a06e4271afdaca8ffce008bed9103b8/test/mjsunit/regress/regress-charat-empty.js

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 30

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6d36bae42c7c866e8bc6de936ee4979c1a2afb2d

commit 6d36bae42c7c866e8bc6de936ee4979c1a2afb2d
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Tue Jan 30 05:10:53 2018

[js-perf-tests] Improve string benchmarks

Add inbounds benchmark for String.p.charCodeAt
and add in and out of bounds benchmarks for
String.p.codePointAt.

Bug: v8:7092,  v8:7326 , chromium:806758
Change-Id: I48065627bd79d8fb24e55b2f6dce590e7adbbd6e
Reviewed-on: https://chromium-review.googlesource.com/891858
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50942}
[modify] https://crrev.com/6d36bae42c7c866e8bc6de936ee4979c1a2afb2d/test/js-perf-test/Strings/string-indexof.js

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 31

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/51c631563895ff9e2b2e73c1f09116f92256a4c6

commit 51c631563895ff9e2b2e73c1f09116f92256a4c6
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Wed Jan 31 17:34:06 2018

[js-perf-tests] Hook up new benchmarks for String.p.charCodeAt

Bug: v8:7092,  v8:7326 , chromium:806758
Change-Id: Id8a3bc2455875af9dfdc01619d8217e033099e7e
Reviewed-on: https://chromium-review.googlesource.com/895690
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51006}
[modify] https://crrev.com/51c631563895ff9e2b2e73c1f09116f92256a4c6/test/js-perf-test/JSTests.json
[modify] https://crrev.com/51c631563895ff9e2b2e73c1f09116f92256a4c6/test/js-perf-test/Strings/string-indexof.js

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 1

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/80447cff70506d3ae333220c08efd3fc1d927bf9

commit 80447cff70506d3ae333220c08efd3fc1d927bf9
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Thu Mar 01 13:52:44 2018

[js-perf-test] Add regression benchmark

This CL adds a regression benchmark for a fast-path of
String.p.charCodeAt, which is important for node.js.

Bug:  v8:7326 
Change-Id: I54efaa2988c595dd40e6a55a3464b3ee7de6f07b
Reviewed-on: https://chromium-review.googlesource.com/942885
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51654}
[modify] https://crrev.com/80447cff70506d3ae333220c08efd3fc1d927bf9/test/js-perf-test/JSTests.json
[modify] https://crrev.com/80447cff70506d3ae333220c08efd3fc1d927bf9/test/js-perf-test/Strings/harmony-string.js

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 2

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b8bc26d09961a99649ed42fa949dac92698683b1

commit b8bc26d09961a99649ed42fa949dac92698683b1
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Fri Mar 02 08:45:30 2018

[turbofan] Preserve order of compares in switches

This CL makes sure that control flow optimization does
not change the order of switches that ultimately get
lowered to a series of comparisons anyway.

Bug:  v8:7326 
Change-Id: If004de6b71a7e9504d37754c847ca108a64e49db
Reviewed-on: https://chromium-review.googlesource.com/941952
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51679}
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/common-operator.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/common-operator.h
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/control-flow-optimizer.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/instruction-selector-impl.h
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/instruction-selector.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/instruction-selector.h
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/mips/instruction-selector-mips.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/mips64/instruction-selector-mips64.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/ppc/instruction-selector-ppc.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/s390/instruction-selector-s390.cc
[modify] https://crrev.com/b8bc26d09961a99649ed42fa949dac92698683b1/src/compiler/x64/instruction-selector-x64.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 2

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9ca27e182245f262db40bc880ebafcaf0bba14ae

commit 9ca27e182245f262db40bc880ebafcaf0bba14ae
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Fri Mar 02 09:26:32 2018

Revert "[turbofan] Preserve order of compares in switches"

This reverts commit b8bc26d09961a99649ed42fa949dac92698683b1.

Reason for revert: may break node.js integration

Original change's description:
> [turbofan] Preserve order of compares in switches
> 
> This CL makes sure that control flow optimization does
> not change the order of switches that ultimately get
> lowered to a series of comparisons anyway.
> 
> Bug:  v8:7326 
> Change-Id: If004de6b71a7e9504d37754c847ca108a64e49db
> Reviewed-on: https://chromium-review.googlesource.com/941952
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51679}

TBR=jarin@chromium.org,sigurds@chromium.org,bmeurer@chromium.org

Change-Id: Ideb551e0831c686dc7c247b77f59ff3485c30181
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  v8:7326 
Reviewed-on: https://chromium-review.googlesource.com/945768
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51681}
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/common-operator.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/common-operator.h
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/control-flow-optimizer.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/instruction-selector-impl.h
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/instruction-selector.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/instruction-selector.h
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/mips/instruction-selector-mips.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/mips64/instruction-selector-mips64.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/ppc/instruction-selector-ppc.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/s390/instruction-selector-s390.cc
[modify] https://crrev.com/9ca27e182245f262db40bc880ebafcaf0bba14ae/src/compiler/x64/instruction-selector-x64.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 2

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/2daca1c6a86af03cfae979e5218bbe06de3f173b

commit 2daca1c6a86af03cfae979e5218bbe06de3f173b
Author: Sigurd Schneider <sigurds@chromium.org>
Date: Fri Mar 02 12:03:42 2018

Reland "[turbofan] Preserve order of compares in switches"

This is a reland of b8bc26d09961a99649ed42fa949dac92698683b1

Original change's description:
> [turbofan] Preserve order of compares in switches
> 
> This CL makes sure that control flow optimization does
> not change the order of switches that ultimately get
> lowered to a series of comparisons anyway.
> 
> Bug:  v8:7326 
> Change-Id: If004de6b71a7e9504d37754c847ca108a64e49db
> Reviewed-on: https://chromium-review.googlesource.com/941952
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51679}

Bug:  v8:7326 
Change-Id: Ifbe61dece499c98bbd49fa3ae9b99ccf4e955ddc
Reviewed-on: https://chromium-review.googlesource.com/945770
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51691}
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/common-operator-reducer.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/common-operator.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/common-operator.h
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/control-flow-optimizer.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/instruction-selector-impl.h
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/instruction-selector.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/instruction-selector.h
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/mips/instruction-selector-mips.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/mips64/instruction-selector-mips64.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/ppc/instruction-selector-ppc.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/s390/instruction-selector-s390.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/test/unittests/compiler/common-operator-unittest.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/test/unittests/compiler/control-flow-optimizer-unittest.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/test/unittests/compiler/node-test-utils.cc
[modify] https://crrev.com/2daca1c6a86af03cfae979e5218bbe06de3f173b/test/unittests/compiler/node-test-utils.h

Status: Verified (was: Assigned)
We're close to crankshaft on the micro-benchmark now. ToT shows:

$ d8-ToT bench.js --nountrusted-code-mitigations
Time: 304 ms.

$ d8-5.8.283.38 bench.js 
Time: 285 ms.



Sign in to add a comment