RESOLVED FIXED 204404
Regression (r252680): JSCOnly build broken: no matching function for call to JSC::DFG::SpeculativeJIT::jsValueResult
https://bugs.webkit.org/show_bug.cgi?id=204404
Summary Regression (r252680): JSCOnly build broken: no matching function for call to ...
Aakash Jain
Reported 2019-11-20 03:22:42 PST
JSCOnly build seems to be broken currently (for ARMv7 and MIPS32el). From https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/10050: FAILED: Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT.cpp.o /home/buildbot/buildroot/buildroot/output.rpi3/host/usr/ccache/arm-buildroot-linux-gnueabihf-g++ ... -c ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compileIncOrDec(JSC::DFG::Node*)’: ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4625:37: error: no matching function for call to ‘JSC::DFG::SpeculativeJIT::jsValueResult(JSC::GPRReg, JSC::DFG::Node*&)’ jsValueResult(result.gpr(), node);
Attachments
Patch (12.75 KB, patch)
2019-11-20 04:30 PST, Caio Lima
no flags
Patch (14.10 KB, patch)
2019-11-20 04:48 PST, Caio Lima
no flags
Patch (14.76 KB, patch)
2019-11-20 07:39 PST, Caio Lima
no flags
Aakash Jain
Comment 2 2019-11-20 03:32:39 PST
Almost certainly https://trac.webkit.org/changeset/252680/webkit That commit modified DFGSpeculativeJIT.cpp. More specifically it added jsValueResult() call inside SpeculativeJIT(). The landed patch wasn't run through EWS, and EWS had few complaints about previous patches in that bug (e.g.: JSC EWS complained about failing JSC tests). Unfortunately JSCOnly EWS from 'old EWS' wasn't run on this patch as this queue was behind.
Caio Lima
Comment 3 2019-11-20 04:04:13 PST
There are 2 Patches that broke builds https://trac.webkit.org/changeset/252684 and https://trac.webkit.org/changeset/252680 The fix to https://trac.webkit.org/changeset/252680 is trivial, but I'm working on a proper fix to https://trac.webkit.org/changeset/252684 in https://bugs.webkit.org/show_bug.cgi?id=204082. I'll try to fix both issues there. If it takes longer than expected, I'll create a patch to fix it ASAP.
Caio Lima
Comment 4 2019-11-20 04:30:24 PST
Created attachment 383955 [details] Patch It fixes the build issues, but it won't have desired behavior until we fix https://bugs.webkit.org/show_bug.cgi?id=204082
Caio Lima
Comment 5 2019-11-20 04:34:12 PST
(In reply to Caio Lima from comment #4) > Created attachment 383955 [details] > Patch > > It fixes the build issues, but it won't have desired behavior until we fix > https://bugs.webkit.org/show_bug.cgi?id=204082 I uploaded the wrong diff.
Caio Lima
Comment 6 2019-11-20 04:48:18 PST
Aakash Jain
Comment 7 2019-11-20 05:53:53 PST
rs=me
Caio Lima
Comment 8 2019-11-20 07:39:28 PST
Saam Barati
Comment 9 2019-11-20 08:14:43 PST
Comment on attachment 383963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383963&action=review r=me. I’ll let you decide if you want to wait on having the baseline use the IC. > Source/JavaScriptCore/bytecode/AccessCase.cpp:1154 > + jit.loadValue(CCallHelpers::BaseIndex(scratchGPR, scratch2GPR, CCallHelpers::TimesEight), JSValueRegs::payloadOnly(scratchGPR)); This won’t work for 32-bit. Is the goal just to get it to compile? > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:-149 > - callOperation(operationGetByVal, dst, m_codeBlock->globalObject(), JSValueRegs(regT1, regT0), JSValueRegs(regT3, regT2)); Why not wait to do this until it all works?
Caio Lima
Comment 10 2019-11-20 09:04:08 PST
Comment on attachment 383963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383963&action=review Thank you very much for the review! >> Source/JavaScriptCore/bytecode/AccessCase.cpp:1154 >> + jit.loadValue(CCallHelpers::BaseIndex(scratchGPR, scratch2GPR, CCallHelpers::TimesEight), JSValueRegs::payloadOnly(scratchGPR)); > > This won’t work for 32-bit. Is the goal just to get it to compile? Yes. I already fixed it to 32-bit locally on GetByVal IC changes. >> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:-149 >> - callOperation(operationGetByVal, dst, m_codeBlock->globalObject(), JSValueRegs(regT1, regT0), JSValueRegs(regT3, regT2)); > > Why not wait to do this until it all works? This also caused a compilation error, since it requires we move `operationGetByVal` out of "DFGOperations.h" or include this header in this file. I think that falling back to slowpath always is a better solution to make it compile.
WebKit Commit Bot
Comment 11 2019-11-20 09:48:46 PST
Comment on attachment 383963 [details] Patch Clearing flags on attachment: 383963 Committed r252690: <https://trac.webkit.org/changeset/252690>
WebKit Commit Bot
Comment 12 2019-11-20 09:48:48 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13 2019-11-20 09:49:18 PST
Note You need to log in before you can comment on or make changes to this bug.