122739 – FTL should use llvm.webkit.patchpoint for inline caches

Bug 122739 - FTL should use llvm.webkit.patchpoint for inline caches

Summary: FTL should use llvm.webkit.patchpoint for inline caches

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Filip Pizlo

URL:
Keywords:

Depends on:	122740 122939 123071 123076 123085 123164
Blocks:	122318
	Show dependency tree / graph

Reported:	2013-10-13 21:53 PDT by Filip Pizlo
Modified:	2013-11-02 20:14 PDT (History)
CC List:	12 users (show)

See Also:

Attachments
it begins (8.40 KB, patch) 2013-10-19 21:41 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
so much fun still to be had! (31.40 KB, patch) 2013-10-21 14:20 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more! (39.92 KB, patch) 2013-10-21 16:58 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
slow path call stuff is written (44.58 KB, patch) 2013-10-21 19:13 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
I think I wrote all of the code (123.60 KB, patch) 2013-10-21 21:09 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
for real this time (58.12 KB, patch) 2013-10-21 21:13 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
starting to compile (68.68 KB, patch) 2013-10-21 21:27 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
it compiles (74.73 KB, patch) 2013-10-21 22:50 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
starting to work? (76.31 KB, patch) 2013-10-21 23:32 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Filip Pizlo 2013-10-13 21:53:48 PDT

...

Comment 1 Filip Pizlo 2013-10-19 21:41:07 PDT

Created attachment 214690 [details]
it begins

But every time I think I can start rage-hacking this into existence, I find another set of prerequisites that I have to handle in another patch.

Comment 2 Filip Pizlo 2013-10-21 14:20:07 PDT

Created attachment 214779 [details]
so much fun still to be had!

Comment 3 Filip Pizlo 2013-10-21 16:58:47 PDT

Created attachment 214795 [details]
more!

Comment 4 Filip Pizlo 2013-10-21 19:13:30 PDT

Created attachment 214806 [details]
slow path call stuff is written

Comment 5 Filip Pizlo 2013-10-21 21:09:00 PDT

Created attachment 214811 [details]
I think I wrote all of the code

Comment 6 Filip Pizlo 2013-10-21 21:13:57 PDT

Created attachment 214812 [details]
for real this time

Haven't tried compiling it yet.

Comment 7 Filip Pizlo 2013-10-21 21:27:20 PDT

Created attachment 214813 [details]
starting to compile

Comment 8 Filip Pizlo 2013-10-21 22:50:12 PDT

Created attachment 214819 [details]
it compiles

Comment 9 Filip Pizlo 2013-10-21 23:32:37 PDT

Created attachment 214821 [details]
starting to work?

Comment 10 WebKit Commit Bot 2013-10-22 03:53:19 PDT

Attachment 214821 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/assembler/AbstractMacroAssembler.h', u'Source/JavaScriptCore/assembler/LinkBuffer.cpp', u'Source/JavaScriptCore/assembler/LinkBuffer.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCapabilities.cpp', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLInlineCacheDescriptor.h', u'Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp', u'Source/JavaScriptCore/ftl/FTLInlineCacheSize.h', u'Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.cpp', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLocation.cpp', u'Source/JavaScriptCore/ftl/FTLLocation.h', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLSlowPathCall.cpp', u'Source/JavaScriptCore/ftl/FTLSlowPathCall.h', u'Source/JavaScriptCore/ftl/FTLSlowPathCallKey.cpp', u'Source/JavaScriptCore/ftl/FTLSlowPathCallKey.h', u'Source/JavaScriptCore/ftl/FTLStackMaps.cpp', u'Source/JavaScriptCore/ftl/FTLStackMaps.h', u'Source/JavaScriptCore/ftl/FTLState.h', u'Source/JavaScriptCore/ftl/FTLThunks.cpp', u'Source/JavaScriptCore/ftl/FTLThunks.h', u'Source/JavaScriptCore/jit/CCallHelpers.h', u'Source/JavaScriptCore/jit/GPRInfo.h', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.h', u'Source/JavaScriptCore/jit/RegisterSet.cpp', u'Source/JavaScriptCore/jit/RegisterSet.h', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/wtf/BitVector.cpp', u'Source/WTF/wtf/BitVector.h', u'Source/WTF/wtf/HashTraits.h', u'Source/WTF/wtf/StdLibExtras.h']" exit_code: 1
Source/WTF/wtf/BitVector.h:271:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/JavaScriptCore/ftl/FTLSlowPathCall.h:60:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/ftl/FTLSlowPathCall.cpp:41:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 3 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 11 Build Bot 2013-10-22 04:34:39 PDT

Comment on attachment 214821 [details]
starting to work?

Attachment 214821 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/8858264

Comment 12 Filip Pizlo 2013-10-22 08:58:54 PDT

This is going to take some effort to get right.  Just as I did with OSR, I think I'll first land the prototype that can do some simple inline cache and then file bugs for the things that need to be done before this passes all tests.

Comment 13 Filip Pizlo 2013-10-23 13:45:34 PDT

This looks like it works as of http://trac.webkit.org/changeset/157872.

Leaving this bug open since the corresponding LLVM support hasn't landed yet.

Comment 14 Filip Pizlo 2013-11-02 20:14:35 PDT

It's in trunk.  The remaining work to port the implementation to the slight changes between the original LLVM patch and what landed will be covered in https://bugs.webkit.org/show_bug.cgi?id=122318.