Earliest revision that I see it in is r187107.
[pizlo@dethklok OpenSource] DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ until_fail.rb WebKitBuild/Release/jsc --useFTLJIT\=false --enableFunctionDotArguments\=true --useFTLJIT\=true --ftlCrashesIfCantInitializeLLVM\=true --thresholdForJITAfterWarmUp\=10 --thresholdForJITSoon\=10 --thresholdForOptimizeAfterWarmUp\=20 --thresholdForOptimizeAfterLongWarmUp\=20 --thresholdForOptimizeSoon\=20 --thresholdForFTLOptimizeAfterWarmUp\=20 --thresholdForFTLOptimizeSoon\=20 --maximumEvalCacheableSourceLength\=150000 PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js Doing run #1... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #2... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #3... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #4... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #5... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #6... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #7... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #8... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #9... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #10... Running ["WebKitBuild/Release/jsc", "--useFTLJIT=false", "--enableFunctionDotArguments=true", "--useFTLJIT=true", "--ftlCrashesIfCantInitializeLLVM=true", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Exception: TypeError: this.initialize.apply is not a function. (In 'this.initialize.apply(this, arguments)', 'this.initialize.apply' is undefined) PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:31:28 testIntersection@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:677:55 rayTrace@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:768:51 getPixelColor@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:669:38 renderScene@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:656:43 renderScene@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:898:26 global code@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:902:14
This worked for me too: DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ until_fail.rb WebKitBuild/Release/jsc "--thresholdForJITAfterWarmUp=10" "--thresholdForJITSoon=10" "--thresholdForOptimizeAfterWarmUp=20" "--thresholdForOptimizeAfterLongWarmUp=20" "--thresholdForOptimizeSoon=20" "--thresholdForFTLOptimizeAfterWarmUp=20" "--thresholdForFTLOptimizeSoon=20" "--maximumEvalCacheableSourceLength=150000" --enableObjectAllocationSinking=false PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js I last tested this on r188482 on Yosemite.
(In reply to comment #2) > This worked for me too: > > DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ until_fail.rb > WebKitBuild/Release/jsc "--thresholdForJITAfterWarmUp=10" > "--thresholdForJITSoon=10" "--thresholdForOptimizeAfterWarmUp=20" > "--thresholdForOptimizeAfterLongWarmUp=20" "--thresholdForOptimizeSoon=20" > "--thresholdForFTLOptimizeAfterWarmUp=20" "--thresholdForFTLOptimizeSoon=20" > "--maximumEvalCacheableSourceLength=150000" > --enableObjectAllocationSinking=false > PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js > > I last tested this on r188482 on Yosemite. I lied. The command line I used was: [pizlo@shakezilla OpenSource] DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ until_fail.rb WebKitBuild/Release/jsc "--thresholdForJITAfterWarmUp=10" "--thresholdForJITSoon=10" "--thresholdForOptimizeAfterWarmUp=20" "--thresholdForOptimizeAfterLongWarmUp=20" "--thresholdForOptimizeSoon=20" "--thresholdForFTLOptimizeAfterWarmUp=20" "--thresholdForFTLOptimizeSoon=20" "--maximumEvalCacheableSourceLength=150000" PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js Doing run #1... Running ["WebKitBuild/Release/jsc", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #2... Running ["WebKitBuild/Release/jsc", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Doing run #3... Running ["WebKitBuild/Release/jsc", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js"] Exception: TypeError: this.initialize.apply is not a function. (In 'this.initialize.apply(this, arguments)', 'this.initialize.apply' is undefined) PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:31:28 initialize@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:527:46 PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:31:28 intersect@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:474:55 testIntersection@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:684:43 rayTrace@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:768:51 rayTrace@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:745:45 getPixelColor@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:669:38 renderScene@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:656:43 renderScene@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:898:26 global code@PerformanceTests/SunSpider/tests/v8-v6/v8-raytrace.js:902:14 /Users/pizlo/bin/until_fail.rb:8:in `block in <main>': fail after 3 runs: #<Process::Status: pid 2950 exit 3> (RuntimeError) from /Users/pizlo/bin/until_fail.rb:4:in `loop' from /Users/pizlo/bin/until_fail.rb:4:in `<main>'
<rdar://problem/22293750>
Created attachment 259059 [details] Patch
I locally changed any reference to "sinking MultiGetByOffset" into "lowering MultiGetByOffset" in the ChangeLog.
Comment on attachment 259059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259059&action=review r=me. > Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js:2 > +Foo.prototype.f = 42; should all "f"s be 42? > Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js:24 > + throw new Error("Result should be 12 but was " + result); "result should be 12" => "result should be 42"
(In reply to comment #7) > Comment on attachment 259059 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259059&action=review > > r=me. > > > Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js:2 > > +Foo.prototype.f = 42; > > should all "f"s be 42? Yes! In current trunk, the output of this test is: > Exception: Error: Result should be 12 but was 1927 I realize this test probably looks super confusing, I'll add a reference before landing stating that it is a regression test for this bug so that people can find the following explanation. > function get(o, p) { > if (p) > return o.f; > return 42; > } > > for (var i = 0; i < 100000; ++i) { > get({ f: 42 }, i % 2); > get({ o: 10, f: 42 }, i % 2); > } This forces the "o.f" access to be profiled as always using either a {f:0} or {o:0,f:1} structure. > function foo() { > var o = new Foo(); > return get(o, isFinalTier()); > } > noInline(foo); > > for (var i = 0; i < 1000000; ++i) { > var result = foo(); > if (result !== 42) > throw new Error("Result should be 12 but was " + result); > } Due to the forced profiling, and since before getting to the FTL the second parameter to get() is always false (i.e. the "o.f" access never gets profiled with a Foo object), the graph for foo looks like this (isFinalTier() being an intrinsic, it is constant-folded to true when compiling for the FTL): <1> NewObject([prototype={f:0}]) <2> MultiGetByOffset(@1, f, [{f:0},{o:0,f:1}]) <3> Return(@2) Here, we will fail the structure check at @2, and OSR exit. With allocation sinking enabled however, we currently make the (wrong) assumption that a MultiGetByOffset can't ever exit, and that (much like a GetByOffset) it must have been protected by a previous CheckStructure. This is obviously wrong, and will lead to the graph being transformed into something like this (1927 is a dummy default value that should never be read and is only present so that we can build valid Phi graphs): <4> JSConstant(1927) <1> PhantomNewObject() <5> JSConstant([prototype={f:0}]) <6> PutHint(@1, StructurePLoc, @5) <7> PutHint(@2, "f", @0) <2> Check() <3> Return(@4) And now we're dead, because instead of exiting @2 is a no-op and we return an out-of-thin-air 1927. With this patch, we will instead generate the following (correct) graph: <4> JSConstant(1927) <1> PhantomNewObject() <5> JSConstant([prototype={f:0}]) <6> PutHint(@1, StructurePLoc, @5) <7> PutHint(@2, "f", @0) <2> CheckStructureImmediate(@5, []) <3> Return(@4) > > > Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js:24 > > + throw new Error("Result should be 12 but was " + result); > > "result should be 12" => "result should be 42" Good catch, I'll change that.
Created attachment 259080 [details] Patch for landing
Comment on attachment 259080 [details] Patch for landing Clearing flags on attachment: 259080 Committed r188507: <http://trac.webkit.org/changeset/188507>
All reviewed patches have been landed. Closing bug.
(In reply to comment #8) > (In reply to comment #7) > > Comment on attachment 259059 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=259059&action=review > > > > r=me. > > > > > Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js:2 > > > +Foo.prototype.f = 42; > > > > should all "f"s be 42? > > Yes! In current trunk, the output of this test is: > > > Exception: Error: Result should be 12 but was 1927 > > I realize this test probably looks super confusing, I'll add a reference > before landing stating that it is a regression test for this bug so that > people can find the following explanation. > > > function get(o, p) { > > if (p) > > return o.f; > > return 42; > > } > > > > for (var i = 0; i < 100000; ++i) { > > get({ f: 42 }, i % 2); > > get({ o: 10, f: 42 }, i % 2); > > } > > This forces the "o.f" access to be profiled as always using either a {f:0} > or {o:0,f:1} structure. > > > function foo() { > > var o = new Foo(); > > return get(o, isFinalTier()); > > } > > noInline(foo); > > > > for (var i = 0; i < 1000000; ++i) { > > var result = foo(); > > if (result !== 42) > > throw new Error("Result should be 12 but was " + result); > > } > > Due to the forced profiling, and since before getting to the FTL the second > parameter to get() is always false (i.e. the "o.f" access never gets > profiled with a Foo object), the graph for foo looks like this > (isFinalTier() being an intrinsic, it is constant-folded to true when > compiling for the FTL): > > <1> NewObject([prototype={f:0}]) > <2> MultiGetByOffset(@1, f, [{f:0},{o:0,f:1}]) > <3> Return(@2) > > Here, we will fail the structure check at @2, and OSR exit. > With allocation sinking enabled however, we currently make the (wrong) > assumption that a MultiGetByOffset can't ever exit, and that (much like a > GetByOffset) it must have been protected by a previous CheckStructure. This > is obviously wrong, and will lead to the graph being transformed into > something like this (1927 is a dummy default value that should never be read > and is only present so that we can build valid Phi graphs): > > <4> JSConstant(1927) > <1> PhantomNewObject() > <5> JSConstant([prototype={f:0}]) > <6> PutHint(@1, StructurePLoc, @5) > <7> PutHint(@2, "f", @0) > <2> Check() > <3> Return(@4) > > And now we're dead, because instead of exiting @2 is a no-op and we return > an out-of-thin-air 1927. > > With this patch, we will instead generate the following (correct) graph: > > > <4> JSConstant(1927) > <1> PhantomNewObject() > <5> JSConstant([prototype={f:0}]) > <6> PutHint(@1, StructurePLoc, @5) > <7> PutHint(@2, "f", @0) > <2> CheckStructureImmediate(@5, []) > <3> Return(@4) Thanks for clarifying. These examples helped me better understand.