New issue
Advanced search Search tips

Issue 664490 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 661577



Sign in to add a comment

Difference between fullcode and turbofan: osr

Project Member Reported by machenb...@chromium.org, Nov 11 2016

Issue description

Happens with --ignition-staging but triggers turbo-osr.

# Minimized program.
// Comment out the next line in ignition and we see "boolean"
var foo = function(msg) {};
foo = function(value) {
  print (typeof value);
}
for (var i = 0; i < 20000; i++) {}
foo(undefined == 0);


# Compared fullcode with ignition_staging

# Flags of fullcode:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging  --random-seed -224729965 --nocrankshaft --turbo-filter=~
# Flags of ignition_staging:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging  --random-seed -224729965 --ignition-staging

Difference:
- boolean
+ number

### Start of configuration fullcode:
boolean

### End of configuration fullcode

### Start of configuration ignition_staging:
number

### End of configuration ignition_staging

 
Components: -Blink>JavaScript Blink>JavaScript>Compiler
Status: Available (was: Untriaged)
Doesn't need OSR. The deoptimizer materializes a number instead of a boolean for the comparison. Here is a simplified repro ...

var foo = function(msg) {}; 
foo = function(value) { 
  print(value);
}
function f() {
  foo(undefined == 0);
}
// No warm-up of {f} so we deopt with {kInsufficientTypeFeedbackForCall}.
%OptimizeFunctionOnNextCall(f);
f();

Comment 3 by jarin@chromium.org, Nov 14 2016

Owner: jarin@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 14 2016

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

commit 1c9528c4c44e0a1bce2fae9d4e2363420746744d
Author: jarin <jarin@chromium.org>
Date: Mon Nov 14 12:10:29 2016

Revert of [turbofan] Fix deoptimization of boolean bit constants. (patchset #1 id:1 of https://codereview.chromium.org/2495243002/ )

Reason for revert:
Seems to break GC stress.

Original issue's description:
> [turbofan] Fix deoptimization of boolean bit constants.
>
> BUG= chromium:664490 

TBR=bmeurer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:664490 

Review-Url: https://codereview.chromium.org/2502613002
Cr-Commit-Position: refs/heads/master@{#40961}

[modify] https://crrev.com/1c9528c4c44e0a1bce2fae9d4e2363420746744d/src/compiler/code-generator.cc
[modify] https://crrev.com/1c9528c4c44e0a1bce2fae9d4e2363420746744d/src/compiler/simplified-lowering.cc
[delete] https://crrev.com/566728031025e7b8c171f91cb1e0537ebcb0813b/test/mjsunit/compiler/regress-664490.js

Cc: yangguo@chromium.org
Status: Assigned (was: Fixed)
I think the gc stress problem is a generally very flaky debug-scopes test. Please reland. I assume Yang is at it. Can we get a skip?
Cc: jgruber@chromium.org
Jakob kindly offered to skip it for gc-stress until  issue 5619  is addressed.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 14 2016

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

commit 1c1edda7db2c97ca80570038cae6daa304a9dcc0
Author: jgruber <jgruber@chromium.org>
Date: Mon Nov 14 15:02:12 2016

Skip flaky debug-scopes test

BUG= v8:5619 , chromium:664490 

Review-Url: https://codereview.chromium.org/2503463002
Cr-Commit-Position: refs/heads/master@{#40968}

[modify] https://crrev.com/1c1edda7db2c97ca80570038cae6daa304a9dcc0/test/mjsunit/mjsunit.status

Comment 9 by jarin@chromium.org, Nov 15 2016

Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 18 2016

Labels: merge-merged-5.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1c9528c4c44e0a1bce2fae9d4e2363420746744d

commit 1c9528c4c44e0a1bce2fae9d4e2363420746744d
Author: jarin <jarin@chromium.org>
Date: Mon Nov 14 12:10:29 2016

Revert of [turbofan] Fix deoptimization of boolean bit constants. (patchset #1 id:1 of https://codereview.chromium.org/2495243002/ )

Reason for revert:
Seems to break GC stress.

Original issue's description:
> [turbofan] Fix deoptimization of boolean bit constants.
>
> BUG= chromium:664490 

TBR=bmeurer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:664490 

Review-Url: https://codereview.chromium.org/2502613002
Cr-Commit-Position: refs/heads/master@{#40961}

[modify] https://crrev.com/1c9528c4c44e0a1bce2fae9d4e2363420746744d/src/compiler/code-generator.cc
[modify] https://crrev.com/1c9528c4c44e0a1bce2fae9d4e2363420746744d/src/compiler/simplified-lowering.cc
[delete] https://crrev.com/566728031025e7b8c171f91cb1e0537ebcb0813b/test/mjsunit/compiler/regress-664490.js

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 18 2016

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

commit 1c1edda7db2c97ca80570038cae6daa304a9dcc0
Author: jgruber <jgruber@chromium.org>
Date: Mon Nov 14 15:02:12 2016

Skip flaky debug-scopes test

BUG= v8:5619 , chromium:664490 

Review-Url: https://codereview.chromium.org/2503463002
Cr-Commit-Position: refs/heads/master@{#40968}

[modify] https://crrev.com/1c1edda7db2c97ca80570038cae6daa304a9dcc0/test/mjsunit/mjsunit.status

Cc: vogelheim@chromium.org
Why does this have the merge-merged-5.6 label? Which CL got merged?
OK - WAI. I'll write a PSA about that on v8-team.
Labels: v8-foozzie-failure

Sign in to add a comment