Bug 123865

Summary:

IC code should handle the call frame register not being the callFrameRegister

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

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

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

112840

Attachments:

Description	Flags
the patch	none
the patch	ggaren: review+

Filip Pizlo

Reported 2013-11-05 21:36:13 PST

Patch forthcoming.

Attachments
the patch (18.58 KB, patch) 2013-11-05 21:38 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (20.36 KB, patch) 2013-11-05 22:11 PST, Filip Pizlo	ggaren: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2013-11-05 21:38:27 PST

Created attachment 216126 [details] the patch

WebKit Commit Bot

Comment 2 2013-11-05 21:40:44 PST

Attachment 216126 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/bytecode/StructureStubInfo.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/jit/CCallHelpers.h', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.h', u'Source/JavaScriptCore/jit/JITPropertyAccess.cpp', u'Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp', u'Source/JavaScriptCore/jit/Repatch.cpp']" exit_code: 1 Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:60: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:116: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 3 2013-11-05 22:11:52 PST

Created attachment 216133 [details] the patch

WebKit Commit Bot

Comment 4 2013-11-05 22:12:41 PST

Attachment 216133 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/bytecode/StructureStubInfo.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp', u'Source/JavaScriptCore/jit/CCallHelpers.h', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.h', u'Source/JavaScriptCore/jit/JITPropertyAccess.cpp', u'Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp', u'Source/JavaScriptCore/jit/Repatch.cpp']" exit_code: 1 Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:60: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:116: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Geoffrey Garen

Comment 5 2013-11-06 11:15:02 PST

Comment on attachment 216133 [details] the patch r=me StyleBot seems to have a point here.

Filip Pizlo

Comment 6 2013-11-06 11:22:42 PST

(In reply to comment #4) > Attachment 216133 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/bytecode/StructureStubInfo.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp', u'Source/JavaScriptCore/jit/CCallHelpers.h', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp', u'Source/JavaScriptCore/jit/JITInlineCacheGenerator.h', u'Source/JavaScriptCore/jit/JITPropertyAccess.cpp', u'Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp', u'Source/JavaScriptCore/jit/Repatch.cpp']" exit_code: 1 > Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:60: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] > Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:116: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] > Total errors found: 2 in 12 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. Really? A PutById/GetById has two arguments: a base and a value. Stylebot wants me to say that the gpr's that I pass to this are: ..., GPRReg base, GPRReg, ... Is that *really* more readable? How is *not* documenting the purpose of that GPRReg argument better than: ..., GPRReg base, GPRReg value, ...

Geoffrey Garen

Comment 7 2013-11-06 11:25:59 PST

Sorry, I misread JSValueRegs as being both base and value, when really it's both tag and payload (on 32-bit). So, StyleBoth is back to not having a point here -- returning its batting average to .000.

Filip Pizlo

Comment 8 2013-11-06 21:03:26 PST

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

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