Patch forthcoming.
Created attachment 284186 [details] the patch
Comment on attachment 284186 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=284186&action=review > Source/JavaScriptCore/b3/B3ValueRep.h:69 > + nit: please remove empty spaces here. > Source/JavaScriptCore/b3/B3ValueRep.h:78 > - > + Unnecessary change. Please remove.
Comment on attachment 284186 [details] the patch r=me Can you add some testb3 specific tests that uses a patchpoint and SomeEarlyRegister to ensure that no input has the same register as the output register.
Comment on attachment 284186 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=284186&action=review > Source/JavaScriptCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=160010 Can you also add the radar url here after the bugzilla one? I think it's customary to do that now.
Thanks for the feedback guys! I've got a 100% repro test that I'll also add: var toggle = 0; function bar() { if (toggle ^= 1) return 42; else return {valueOf: function() { return 42; }}; } noInline(bar); function baz() { return 7; } noInline(baz); function foo() { return bar() ^ baz(); } noInline(foo); for (var i = 0; i < 100000; ++i) { var result = foo(); if (result != 45) throw "Error: bad result: " + result; }
(In reply to comment #2) > Comment on attachment 284186 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284186&action=review > > > Source/JavaScriptCore/b3/B3ValueRep.h:69 > > + > > nit: please remove empty spaces here. Fixed. > > > Source/JavaScriptCore/b3/B3ValueRep.h:78 > > - > > + > > Unnecessary change. Please remove. Fixed.
(In reply to comment #3) > Comment on attachment 284186 [details] > the patch > > r=me > Can you add some testb3 specific tests that uses a patchpoint and > SomeEarlyRegister to ensure that no input has the same register as the > output register. Sure! I added a test that runs the same compile in two modes: one in which an arrangement of patchpoints causes some patchpoint to see the same register for input and output because it's presented as an ultra-tempting register coalescing oppportunity, and another in which we use SomeEarlyRegister to prevent coalescing.
(In reply to comment #4) > Comment on attachment 284186 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284186&action=review > > > Source/JavaScriptCore/ChangeLog:4 > > + https://bugs.webkit.org/show_bug.cgi?id=160010 > > Can you also add the radar url here after the bugzilla one? I think it's > customary to do that now. OK!
Created attachment 284191 [details] patch for landing
Attachment 284191 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:12382: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:12391: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:12404: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 3 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Landed in https://trac.webkit.org/changeset/203488