Bug 123164

Summary:

FTL should be able to do some simple inline caches using LLVM patchpoints

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

atrick, barraclough, benjamin, cmarcelo, commit-queue, ggaren, mark.lam, mhahnenberg, msaboff, nrotem, oliver, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

122739

Attachments:

Description	Flags
the patch	buildbot: commit-queue-
the patch	mhahnenberg: review+, buildbot: commit-queue-
patch for landing	buildbot: commit-queue-
patch for landing	none

Description Filip Pizlo 2013-10-22 09:00:44 PDT

This tracks the initial prototype of patchpoint-based inline caches.

Comment 1 Filip Pizlo 2013-10-22 09:06:14 PDT

Created attachment 214860 [details]
the patch

Comment 2 WebKit Commit Bot 2013-10-22 09:08:19 PDT

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', 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/ChangeLog', 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 39 files


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

Comment 3 Filip Pizlo 2013-10-22 09:10:29 PDT

Comment on attachment 214860 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=214860&action=review

>> Source/JavaScriptCore/ftl/FTLSlowPathCall.cpp:41
>> +        
> 
> When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]

Fixed.

>> Source/JavaScriptCore/ftl/FTLSlowPathCall.h:60
>> +    GPRReg result, GPRReg callFrameRegister, StructureStubInfo*, GPRReg object,
> 
> The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]

No.

>> Source/WTF/wtf/BitVector.h:271
>> +        if (sizeof(uintptr_t) == 4)
> 
> An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]

Lame.  Fixed.

Comment 4 Build Bot 2013-10-22 09:53:59 PDT

Comment on attachment 214860 [details]
the patch

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

Comment 5 Filip Pizlo 2013-10-22 14:45:47 PDT

Created attachment 214886 [details]
the patch

rebased.

Comment 6 WebKit Commit Bot 2013-10-22 14:48:59 PDT

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', 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/ChangeLog', 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 39 files


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

Comment 7 Mark Hahnenberg 2013-10-22 15:23:44 PDT

Comment on attachment 214886 [details]
the patch

r=me

Comment 8 Build Bot 2013-10-22 15:31:50 PDT

Comment on attachment 214886 [details]
the patch

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

Comment 9 Nadav Rotem 2013-10-22 17:42:28 PDT

// This code relies on us being 64-bit. FTL is currently always 64-bit.
static const size_t wordSize = 8;

Can you add an assertion ?


 82        size_t stackAlignment = 16;
 
Is this the right place for this constant ?

Comment 10 Filip Pizlo 2013-10-22 17:49:26 PDT

(In reply to comment #9)
> // This code relies on us being 64-bit. FTL is currently always 64-bit.
> static const size_t wordSize = 8;
> 
> Can you add an assertion ?

I could.  Though there are already a bunch of such assertions throughout the FTL.

> 
> 
>  82        size_t stackAlignment = 16;
> 
> Is this the right place for this constant ?

Maybe, maybe not?  We don't currently have anywhere to put it.

Comment 11 Filip Pizlo 2013-10-22 18:10:24 PDT

Created attachment 214916 [details]
patch for landing

Comment 12 WebKit Commit Bot 2013-10-22 18:11:31 PDT

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', 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/ChangeLog', 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 39 files


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

Comment 13 Filip Pizlo 2013-10-22 18:12:21 PDT

(In reply to comment #12)
> Attachment 214916 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', 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/ChangeLog', 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]

Oops, I will fix 2/3 of these, I promise.

> Total errors found: 3 in 39 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 14 Build Bot 2013-10-22 18:52:27 PDT

Comment on attachment 214916 [details]
patch for landing

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

Comment 15 Filip Pizlo 2013-10-22 18:54:33 PDT

Created attachment 214918 [details]
patch for landing

Comment 16 WebKit Commit Bot 2013-10-22 19:29:42 PDT

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', 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/ChangeLog', 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/JavaScriptCore/ftl/FTLSlowPathCall.h:60:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 39 files


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

Comment 17 Filip Pizlo 2013-10-23 11:20:56 PDT

Landed in http://trac.webkit.org/changeset/157872