[Webkit-unassigned] [Bug 160588] Refactor MathIC compilation process in Baseline and DFG to turn temporary registers usage more flexible

Wed Sep 21 00:16:39 PDT 2016

https://bugs.webkit.org/show_bug.cgi?id=160588

--- Comment #5 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 288391
  --> https://bugs.webkit.org/attachment.cgi?id=288391
Patch

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

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3457
>> +    for (int i = 0; i < MATH_IC_MAX_GPR; i++)
> 
> should be uint32_t or size_t. (For all loops below, too.)

Done.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3482
>> +    if (scratchGPRRegCount == 1)
> 
> Hmm, this doesn't like it'd scale well.

I agree, however I am getting the following error because there is no sufficient register:

sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: ASSERTION FAILED: currentLowest != NUM_REGS && currentSpillOrder != SpillHintInvalid
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: /Users/caiolima/open_projects/WebKit/Source/JavaScriptCore/dfg/DFGRegisterBank.h(138) : RegID JSC::DFG::RegisterBank<JSC::GPRInfo>::allocate(JSC::VirtualRegister &) [BankInfo = JSC::GPRInfo]
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 1   0xe96ffa WTFCrash
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 2   0x6cf606 JSC::DFG::RegisterBank<JSC::GPRInfo>::allocate(JSC::VirtualRegister&)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 3   0x6b45f5 JSC::DFG::SpeculativeJIT::allocate()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 4   0x6fe7cd JSC::DFG::SpeculativeJIT::fillJSValue(JSC::DFG::Edge, JSC::X86Registers::RegisterID&, JSC::X86Registers::RegisterID&, JSC::X86Registers::XMMRegisterID&)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 5   0x6d18c2 JSC::DFG::JSValueOperand::fill()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 6   0x6b9c35 JSC::DFG::JSValueOperand::tagGPR()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 7   0x6b4cda JSC::DFG::JSValueOperand::jsValueRegs()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 8   0x6c4764 void JSC::DFG::SpeculativeJIT::compileMathIC<JSC::JITMulGenerator, long long (*)(JSC::ExecState*, long long, long long, JSC::JITMathIC<JSC::JITMulGenerator>*), long long (*)(JSC::ExecState*, long long, long long)>(JSC::DFG::Node*, JSC::JITMathIC<JSC::JITMulGenerator>*, unsigned char, unsigned char, long long (*)(JSC::ExecState*, long long, long long, JSC::JITMathIC<JSC::JITMulGenerator>*), long long (*)(JSC::ExecState*, long long, long long))
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 9   0x6926f2 JSC::DFG::SpeculativeJIT::compileArithMul(JSC::DFG::Node*)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 10  0x70ea82 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 11  0x68130f JSC::DFG::SpeculativeJIT::compileCurrentBlock()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 12  0x681c9f JSC::DFG::SpeculativeJIT::compile()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 13  0x5264bc JSC::DFG::JITCompiler::compileBody()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 14  0x52a08a JSC::DFG::JITCompiler::compileFunction()
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 15  0x631f70 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 16  0x630a8a JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 17  0x4a6280 JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::PassRefPtr<JSC::DeferredCompilationCallback>)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 18  0x4a5c62 JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::PassRefPtr<JSC::DeferredCompilationCallback>)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 19  0x973336 operationOptimize
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 20  0x2b69d27
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 21  0xb63964 llint_entry
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 22  0xb639bb llint_entry
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 23  0x2b5f118
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 24  0xb639bb llint_entry
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 25  0xb5e41c vmEntryToJavaScript
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 26  0x95888f JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 27  0x8f5943 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 28  0x2babb5 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 29  0x54ae8 runWithScripts(GlobalObject*, WTF::Vector<Script, 0ul, WTF::CrashOnOverflow, 16ul> const&, WTF::String const&, bool, bool, bool)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 30  0x53c14 runJSC(JSC::VM*, CommandLine)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: 31  0x52a69 jscmain(int, char**)
sunspider-1.0/3d-raytrace.js.no-cjit-validate-phases: test_script_19: line 2: 78419 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --validateBytecode\=true --validateGraphAtEachPhase\=true --useSourceProviderCache\=false --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 --scribbleFreeCells\=true 3d-raytrace.js )

One way I am thinking now is ASSERT(MATH_IC_MAX_GPR <= 2) for x86_32 cases, However it still doesn't solve our limitation on https://bugs.webkit.org/show_bug.cgi?id=160012 because we need 3 scratchGPRs there. Do you think spill the registers makes sense here?

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:1583
>> +        patchpoint->numFPScratchRegisters = 2 + numFPScratchRegisters;
> 
> The idea of doing this is so that we don't require the 2 extra FP registers when not needed. So the caller should be responsible of providing these numbers.

Done.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160921/276c59e3/attachment-0001.html>