Bug 153046

Summary: The StringFromCharCode DFG intrinsic should support untyped operands.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, keith_miller, msaboff, saam
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 153019    
Attachments:
Description Flags
proposed patch.
ggaren: review+
x86_64 benchmark result.
none
x86 benchmark results. none

Mark Lam
Reported 2016-01-12 16:04:58 PST
The current StringFromCharCode DFG intrinsic assumes that its operand charCode must be an Int32. This results in 26000+ BadType OSR exits in the LongSpider crypto-aes benchmark. With support for Untyped operands, the number of OSR exits drops to 202.
Attachments
proposed patch. (11.51 KB, patch)
2016-01-12 16:21 PST, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
x86_64 benchmark result. (66.04 KB, text/plain)
2016-01-13 10:09 PST, Mark Lam 		no flags
Details
x86 benchmark results. (65.79 KB, text/plain)
2016-01-13 10:10 PST, Mark Lam 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-01-12 16:21:34 PST
Created attachment 268828 [details] proposed patch. The patch passes the JSC tests. However, still need to run benchmarks on them.
Mark Lam
Comment 2 2016-01-13 10:09:48 PST
Created attachment 268872 [details] x86_64 benchmark result.
Mark Lam
Comment 3 2016-01-13 10:10:25 PST
Created attachment 268873 [details] x86 benchmark results.
Mark Lam
Comment 4 2016-01-13 10:28:26 PST
The benchmarks shows that perf is neutral with this patch. All of the differences are due to noise (does not repeat on retry) except for one: ftl-polymorphic-StringFromCharCode which I just added for this patch: ftl-polymorphic-StringFromCharCode 246.9686+-17.4090 ^ 207.8919+-11.0696 ^ definitely 1.1880x faster This is only for 64 bit x86_64 though. It does not manifest on x86 because there appears to be a bug on 32-bit where the StringFromCharCode intrinsic is not recognized and therefore not deployed: https://bugs.webkit.org/show_bug.cgi?id=153066. This patch is ready for a review.
Geoffrey Garen
Comment 5 2016-01-13 14:43:12 PST
Comment on attachment 268828 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=268828&action=review r=me > Source/JavaScriptCore/dfg/DFGOperations.cpp:1316 > +EncodedJSValue JIT_OPERATION operationStringFromValueCharCode(ExecState* exec, EncodedJSValue encodedValue) I would call this operationStringFromCharCodeUntyped. If we have to do this in more cases, we should create an operationToUInt32 instead.
Mark Lam
Comment 6 2016-01-13 15:30:16 PST
Thanks for the review. Landed in r194996: <http://trac.webkit.org/r194996>.
Note You need to log in before you can comment on or make changes to this bug.