Bug 123076 - StructureStubInfo's usedRegisters set should be able to track all registers, not just the ones that our JIT's view as temporaries
Summary: StructureStubInfo's usedRegisters set should be able to track all registers, ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 123077 123079
Blocks: 122739
  Show dependency treegraph
 
Reported: 2013-10-19 20:45 PDT by Filip Pizlo
Modified: 2013-10-21 02:48 PDT (History)
15 users (show)

See Also:


Attachments
work in progress (25.36 KB, patch)
2013-10-20 11:50 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (37.64 KB, patch)
2013-10-20 14:19 PDT, Filip Pizlo
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-10-19 20:45:48 PDT
...
Comment 1 Filip Pizlo 2013-10-20 11:50:54 PDT
Created attachment 214704 [details]
work in progress
Comment 2 Filip Pizlo 2013-10-20 14:19:14 PDT
Created attachment 214706 [details]
the patch

Heh.  It seems to work.
Comment 3 WebKit Commit Bot 2013-10-20 14:20:26 PDT
Attachment 214706 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/assembler/MacroAssembler.h', u'Source/JavaScriptCore/bytecode/StructureStubInfo.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/ftl/FTLSaveRestore.cpp', 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/RegisterSet.cpp', u'Source/JavaScriptCore/jit/RegisterSet.h', u'Source/JavaScriptCore/jit/Repatch.cpp', u'Source/JavaScriptCore/jit/TempRegisterSet.cpp', u'Source/JavaScriptCore/jit/TempRegisterSet.h', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/BitVector.cpp', u'Source/WTF/wtf/BitVector.h']" exit_code: 1
Source/JavaScriptCore/bytecode/StructureStubInfo.h:41:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:59:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Sam Weinig 2013-10-20 17:44:06 PDT
Comment on attachment 214706 [details]
the patch

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

>> Source/JavaScriptCore/bytecode/StructureStubInfo.h:41
>> +#include "RegisterSet.h"
> 
> Alphabetical sorting problem.  [build/include_order] [4]

Sort please.
Comment 5 Filip Pizlo 2013-10-20 21:49:52 PDT
(In reply to comment #4)
> (From update of attachment 214706 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=214706&action=review
> 
> >> Source/JavaScriptCore/bytecode/StructureStubInfo.h:41
> >> +#include "RegisterSet.h"
> > 
> > Alphabetical sorting problem.  [build/include_order] [4]
> 
> Sort please.

Fixed!
Comment 6 Filip Pizlo 2013-10-20 21:50:06 PDT
Landed in http://trac.webkit.org/changeset/157707
Comment 7 Csaba Osztrogonác 2013-10-21 02:48:09 PDT
(In reply to comment #6)
> Landed in http://trac.webkit.org/changeset/157707

FYI: It broke the Apple Windows build:

   1>JITPropertyAccess32_64.obj : error LNK2019: unresolved external symbol "public: static class JSC::RegisterSet __cdecl JSC::RegisterSet::specialRegisters(void)" (?specialRegisters@RegisterSet@JSC@@SA?AV12@XZ) referenced in function "private: void __thiscall JSC::JIT::emit_op_get_by_id(struct JSC::Instruction *)" (?emit_op_get_by_id@JIT@JSC@@AAEXPAUInstruction@2@@Z)
     1>Repatch.obj : error LNK2019: unresolved external symbol "public: __thiscall JSC::TempRegisterSet::TempRegisterSet(class JSC::RegisterSet const &)" (??0TempRegisterSet@JSC@@QAE@ABVRegisterSet@1@@Z) referenced in function "void __cdecl JSC::generateProtoChainAccessStub(class JSC::ExecState *,struct JSC::StructureStubInfo &,class JSC::StructureChain *,unsigned int,int,class JSC::Structure *,class JSC::CodeLocationLabel,class JSC::CodeLocationLabel,class WTF::RefPtr<class JSC::JITStubRoutine> &)" (?generateProtoChainAccessStub@JSC@@YAXPAVExecState@1@AAUStructureStubInfo@1@PAVStructureChain@1@IHPAVStructure@1@VCodeLocationLabel@1@4AAV?$RefPtr@VJITStubRoutine@JSC@@@WTF@@@Z)
     1>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\bin32\JavaScriptCore.dll : fatal error LNK1120: 2 unresolved externals
     1>Done Building Project "C:\cygwin\home\buildbot\slave\win-release\build\Source\JavaScriptCore\JavaScriptCore.vcxproj\JavaScriptCore.vcxproj" (build target(s)) -- FAILED.