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

Filip Pizlo

Reported 2013-10-22 09:00:44 PDT

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

Attachments
the patch (82.65 KB, patch) 2013-10-22 09:06 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (82.61 KB, patch) 2013-10-22 14:45 PDT, Filip Pizlo	mhahnenberg: review+ buildbot: commit-queue-	Details Formatted Diff Diff
patch for landing (82.61 KB, patch) 2013-10-22 18:10 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
patch for landing (82.62 KB, patch) 2013-10-22 18:54 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

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

Created attachment 214860 [details] the patch

WebKit Commit Bot

Comment 2 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.

Filip Pizlo

Comment 3 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.

Build Bot

Comment 4 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

Filip Pizlo

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

Created attachment 214886 [details] the patch rebased.

WebKit Commit Bot

Comment 6 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.

Mark Hahnenberg

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

Comment on attachment 214886 [details] the patch r=me

Build Bot

Comment 8 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

Nadav Rotem

Comment 9 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 ?

Filip Pizlo

Comment 10 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.

Filip Pizlo

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

Created attachment 214916 [details] patch for landing

WebKit Commit Bot

Comment 12 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.

Filip Pizlo

Comment 13 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.

Build Bot

Comment 14 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

Filip Pizlo

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

Created attachment 214918 [details] patch for landing

WebKit Commit Bot

Comment 16 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.

Filip Pizlo

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

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

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